-
-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Create js_run_devserver target in macro #92
base: main
Are you sure you want to change the base?
Conversation
cypress/private/cypress_test.bzl
Outdated
cypress_files = ctx.toolchains["@aspect_rules_cypress//cypress:toolchain_type"].cypressinfo.tool_files | ||
|
||
files = ctx.files.data[:] + cypress_files + ctx.files.browsers | ||
|
||
if ctx.attr.chdir: | ||
cypress_bin = "/".join([".." for _ in ctx.attr.chdir.split("/")] + [cypress_bin]) | ||
# TODO: Augment rules_js to expose EXECROOT before setting fixed_env. Then use that variable. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gregmagolan What do you think about this TODO? Is that a sane change to rules_js? Is this a sane way to lookup the execroot in the first place?
|
||
launcher = js_binary_lib.create_launcher( | ||
ctx, | ||
log_prefix_rule_set = "aspect_rules_cypess", | ||
log_prefix_rule = "cypress_node_test", | ||
fixed_args = ctx.attr.fixed_args, | ||
fixed_env = { | ||
"CYPRESS_RUN_BINARY": cypress_bin, | ||
"HOME": "$$TEST_TMPDIR", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TEST_TMPDIR
isn't defined in bazel run
so I removed these. I'd like to keep them and use something like or
if they aren't defined. Any suggestions @gregmagolan ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did we set HOME
in the first place? Guessing cypress was trying to write to the home directory when it the sandbox and failing?
973b560
to
98c52d1
Compare
For every `cypress_test` the macro now generates a supplementary target `[name].open` which runs cypress using js_run_devserver. This allows users to use ibazel to interactively update their tests.
98c52d1
to
94beda2
Compare
if ctx.attr.chdir: | ||
cypress_bin = "/".join([".." for _ in ctx.attr.chdir.split("/")] + [cypress_bin]) | ||
# TODO: Augment rules_js to expose JS__EXECROOT before setting fixed_env. Then use that variable. | ||
execroot = "$$( realpath \"$${BASH_SOURCE[0]}\" | sed -E 's/^(.*\\/execroot\\/[^/]+).*/\\1/')" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Dep on realpath not great and it will also escape the sandbox this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably augment rules_js to support what you want here instead of this line
Every
cypress_test
the macro now generatesa supplementary target
[name].open
which runscypress using js_run_devserver. This allows users
to use ibazel to interactively update their tests.
Changes are visible to end-users: yes
Test plan
cypress_test
macro now creates another target with name[name].open
. This target runscypress open
using js_run_devserver. It is meant to be run withibazel
so that your tests can be updated interactively.