Skip to content

Commit

Permalink
Review comments addressed
Browse files Browse the repository at this point in the history
- Switch to `disable_sandbox` defaulting to true
- Removed unused attribute from rule impl
  • Loading branch information
mrmeku committed Apr 30, 2024
1 parent 6ca0aeb commit 4baab3c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 15 deletions.
16 changes: 8 additions & 8 deletions cypress/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@ _cypress_test = rule(
toolchains = js_binary_lib.toolchains + ["@aspect_rules_cypress//cypress:toolchain_type"],
)

def _cypress_test_macro(name, entry_point, cypress, **kwargs):
def _cypress_test_macro(name, entry_point, cypress, disable_sandbox, **kwargs):
tags = kwargs.pop("tags", [])
if not kwargs.pop("allow_sandbox", False):
if disable_sandbox:
tags.append("no-sandbox")
_cypress_test(
name = name,
Expand All @@ -38,7 +38,7 @@ def _cypress_test_macro(name, entry_point, cypress, **kwargs):
def cypress_test(
name,
cypress = "//:node_modules/cypress",
allow_sandbox = False,
disable_sandbox = True,
browsers = [],
**kwargs):
"""cypress_test runs the cypress CLI with the cypress toolchain.
Expand All @@ -52,7 +52,7 @@ def cypress_test(
Args:
name: The name used for this rule and output files
cypress: The cypress npm package which was already linked using an API like npm_link_all_packages.
allow_sandbox: Turn off sandboxing by default to allow electron to perform write operations.
disable_sandbox: Turn off sandboxing by default to allow electron to perform write operations.
Cypress does not expose the underlying electron apis so we
cannot alter the user app data directory to be within the bazel
sandbox.
Expand Down Expand Up @@ -92,7 +92,7 @@ def cypress_test(
name = name,
entry_point = entry_point,
cypress = cypress,
allow_sandbox = allow_sandbox,
disable_sandbox = disable_sandbox,
browsers = browsers,
**kwargs
)
Expand All @@ -101,7 +101,7 @@ def cypress_module_test(
name,
runner,
cypress = "//:node_modules/cypress",
allow_sandbox = False,
disable_sandbox = True,
browsers = [],
**kwargs):
"""cypress_module_test creates a node environment which is hooked up to the cypress toolchain.
Expand Down Expand Up @@ -133,7 +133,7 @@ def cypress_module_test(
runner: JS file to call into the cypress module api
See https://docs.cypress.io/guides/guides/module-api
cypress: The cypress npm package which was already linked using an API like npm_link_all_packages.
allow_sandbox: Turn off sandboxing by default to allow electron to perform write operations.
disable_sandbox: Turn off sandboxing by default to allow electron to perform write operations.
Cypress does not expose the underlying electron apis so we
cannot alter the user app data directory to be within the bazel
sandbox.
Expand Down Expand Up @@ -165,7 +165,7 @@ def cypress_module_test(
name = name,
entry_point = runner,
cypress = cypress,
allow_sandbox = allow_sandbox,
disable_sandbox = disable_sandbox,
browsers = browsers,
**kwargs
)
3 changes: 0 additions & 3 deletions cypress/private/cypress_test.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@ load("@aspect_rules_js//js:libs.bzl", "js_binary_lib", "js_lib_helpers")
load("@bazel_skylib//lib:dicts.bzl", "dicts")

_attrs = dicts.add(js_binary_lib.attrs, {
"allow_sandbox": attr.bool(
default = False,
),
"browsers": attr.label_list(
allow_files = True,
),
Expand Down
8 changes: 4 additions & 4 deletions docs/defs.md

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4baab3c

Please sign in to comment.