fix: Disable sandboxing for cypress_test rules (#66)

* Cypress has moved away from supporting the env variable RUN_ELECTRON_AS_NODE which makes it hard to keep up with their electron server's sandboxing violations. We now disable the sandbox so that user's can don't run into these issues. * Review comments addressed - Switch to `disable_sandbox` defaulting to true - Removed unused attribute from rule impl
aspect-build · May 1, 2024 · 6e9bd95 · 6e9bd95
1 parent 7334fbb
commit 6e9bd95
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 44 deletions.
diff --git a/cypress/defs.bzl b/cypress/defs.bzl
@@ -12,7 +12,35 @@ _cypress_test = rule(
     toolchains = js_binary_lib.toolchains + ["@aspect_rules_cypress//cypress:toolchain_type"],
 )
 
-def cypress_test(name, cypress = "//:node_modules/cypress", **kwargs):
+def _cypress_test_macro(name, entry_point, cypress, disable_sandbox, **kwargs):
+    tags = kwargs.pop("tags", [])
+    if disable_sandbox:
+        tags.append("no-sandbox")
+    _cypress_test(
+        name = name,
+        entry_point = entry_point,
+        data = kwargs.pop("data", []) + [
+            cypress,
+        ],
+        chdir = native.package_name(),
+        enable_runfiles = select({
+            "@aspect_rules_js//js/private:enable_runfiles": True,
+            "//conditions:default": False,
+        }),
+        unresolved_symlinks_enabled = select({
+            "@aspect_rules_js//js/private:experimental_allow_unresolved_symlinks": True,
+            "//conditions:default": False,
+        }),
+        tags = tags,
+        **kwargs
+    )
+
+def cypress_test(
+        name,
+        cypress = "//:node_modules/cypress",
+        disable_sandbox = True,
+        browsers = [],
+        **kwargs):
     """cypress_test runs the cypress CLI with the cypress toolchain.
 
     The environment is bootstrapped by first setting the environment variable `CYPRESS_RUN_BINARY` to the binary downloaded by the cypress toolchain. See https://docs.cypress.io/guides/references/advanced-installation#Run-binary
@@ -24,6 +52,32 @@ def cypress_test(name, cypress = "//:node_modules/cypress", **kwargs):
     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.
+        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.
+
+            From https://www.electronjs.org/docs/latest/api/app
+            appData Per-user application data directory, which by default points to:
+                %APPDATA% on Windows
+                $XDG_CONFIG_HOME or ~/.config on Linux
+                ~/Library/Application Support on macOS
+
+            Cypress may fail to connect on macos with errors like:
+                Timed out waiting for the browser to connect. Retrying...
+                Timed out waiting for the browser to connect. Retrying again...
+                The browser never connected. Something is wrong. The tests cannot run. Aborting...
+        browsers: A sequence of labels specifying the browsers to include.
+            Usually, any dependency that you wish to be included in the runfiles tree should
+            be included within the data attribute. However, data dependencies, by default,
+            are copied to the Bazel output tree before being passed as inputs to runfiles.
+
+            This is not a good default behavior for browser since these typically come from
+            external workspaces which cannot be symlinked into bazel-bin. Instead, we
+            place them at the root of the runfiles tree. Use relative paths to construct
+            account for this placement
+
+            e.g. ../../../BROWSER_WORKSPACE_NAME
         **kwargs: All other args from `js_test`. See https://github.com/aspect-build/rules_js/blob/main/docs/js_binary.md#js_test
     """
     entry_point = "%s__entry_point" % name
@@ -34,25 +88,22 @@ def cypress_test(name, cypress = "//:node_modules/cypress", **kwargs):
         tags = ["manual"],
     )
 
-    _cypress_test(
+    _cypress_test_macro(
         name = name,
         entry_point = entry_point,
-        data = kwargs.pop("data", []) + [
-            cypress,
-        ],
-        chdir = native.package_name(),
-        enable_runfiles = select({
-            "@aspect_rules_js//js/private:enable_runfiles": True,
-            "//conditions:default": False,
-        }),
-        unresolved_symlinks_enabled = select({
-            "@aspect_rules_js//js/private:experimental_allow_unresolved_symlinks": True,
-            "//conditions:default": False,
-        }),
+        cypress = cypress,
+        disable_sandbox = disable_sandbox,
+        browsers = browsers,
         **kwargs
     )
 
-def cypress_module_test(name, runner, cypress = "//:node_modules/cypress", **kwargs):
+def cypress_module_test(
+        name,
+        runner,
+        cypress = "//:node_modules/cypress",
+        disable_sandbox = True,
+        browsers = [],
+        **kwargs):
     """cypress_module_test creates a node environment which is hooked up to the cypress toolchain.
 
     The environment is bootstrapped by first setting the environment variable `CYPRESS_RUN_BINARY` to the binary downloaded by the cypress toolchain. See https://docs.cypress.io/guides/references/advanced-installation#Run-binary
@@ -82,23 +133,39 @@ def cypress_module_test(name, runner, cypress = "//:node_modules/cypress", **kwa
         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.
+        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.
+
+            From https://www.electronjs.org/docs/latest/api/app
+            appData Per-user application data directory, which by default points to:
+                %APPDATA% on Windows
+                $XDG_CONFIG_HOME or ~/.config on Linux
+                ~/Library/Application Support on macOS
+
+            Cypress may fail to connect on macos with errors like:
+                Timed out waiting for the browser to connect. Retrying...
+                Timed out waiting for the browser to connect. Retrying again...
+                The browser never connected. Something is wrong. The tests cannot run. Aborting...
+        browsers: A sequence of labels specifying the browsers to include.
+            Usually, any dependency that you wish to be included in the runfiles tree should
+            be included within the data attribute. However, data dependencies, by default,
+            are copied to the Bazel output tree before being passed as inputs to runfiles.
+
+            This is not a good default behavior for browser since these typically come from
+            external workspaces which cannot be symlinked into bazel-bin. Instead, we
+            place them at the root of the runfiles tree. Use relative paths to construct
+            account for this placement
+
+            e.g. ../../../BROWSER_WORKSPACE_NAME
         **kwargs: All other args from `js_test`. See https://github.com/aspect-build/rules_js/blob/main/docs/js_binary.md#js_test
     """
-    _cypress_test(
+    _cypress_test_macro(
         name = name,
-        enable_runfiles = select({
-            "@aspect_rules_js//js/private:enable_runfiles": True,
-            "//conditions:default": False,
-        }),
-        unresolved_symlinks_enabled = select({
-            "@aspect_rules_js//js/private:experimental_allow_unresolved_symlinks": True,
-            "//conditions:default": False,
-        }),
         entry_point = runner,
-        chdir = native.package_name(),
-        data = kwargs.pop("data", []) + [
-            cypress,
-        ],
-        patch_node_fs = kwargs.pop("patch_node_fs", False),
+        cypress = cypress,
+        disable_sandbox = disable_sandbox,
+        browsers = browsers,
         **kwargs
     )
diff --git a/cypress/private/cypress_test.bzl b/cypress/private/cypress_test.bzl
@@ -5,19 +5,6 @@ load("@bazel_skylib//lib:dicts.bzl", "dicts")
 
 _attrs = dicts.add(js_binary_lib.attrs, {
     "browsers": attr.label_list(
-        doc = """A sequence of labels specifying the browsers to include.
-
-Usually, any dependency that you wish to be included in the runfiles tree should
-be included within the data attribute. However, data dependencies, by default,
-are copied to the Bazel output tree before being passed as inputs to runfiles.
-
-This is not a good default behavior for browser since these typically come from
-external workspaces which cannot be symlinked into bazel-bin. Instead, we
-place them at the root of the runfiles tree. Use relative paths to construct
-account for this placement
-
-  e.g. ../../../BROWSER_WORKSPACE_NAME
-""",
         allow_files = True,
     ),
 })

diff --git a/cypress/private/versions.bzl b/cypress/private/versions.bzl
@@ -3,6 +3,34 @@
 # Use /scripts/mirror_release.sh to add a newer version below.
 # Versions should be descending order so TOOL_VERSIONS.keys()[0] is the latest version.
 TOOL_VERSIONS = {
+    "13.7.3": {
+        "darwin-x64": "d24aaadf9e7f014c137fd36efe571e7f5cae3ae0e709b79e4882913f36ac6649",
+        "darwin-arm64": "d96a0ad65967a6c24652f7fb694a56ad593328a7b112943bfd986645577f5b0e",
+        "linux-x64": "6a120720b12e92d427aebaa807f0460d6bac10c32120263c64684f7ff8f906f2",
+        "linux-arm64": "190ea5da85d6341dde1717c49ed1c476bb34a7e1b949f00fb0ac8b244821fc68",
+        "win32-x64": "e9bd6cd3b76ce601812a47203875f5b3d7a68df040bf1d0e102ce83bd61e82df",
+    },
+    "13.7.2": {
+        "darwin-x64": "7e53b9b6ca41a228a2227f00bc90b327109ff5fc903cdd178680034b49da79de",
+        "darwin-arm64": "2b47b4374b7d8544686689a991a891d955cfb9f5e5dc290f2fba519bff7f9640",
+        "linux-x64": "fca0e8d9d0437d05e7ddcc0bba84903386f56212cdea45426a42c973552ede1e",
+        "linux-arm64": "6f181ec4b79d9f961c05b0954ddfa3b81be631c45d7f449792ccbb3c062536dd",
+        "win32-x64": "1aa54723279e64b5bccfe9ca089729a9ec2e355e73cd51d792f3d62472b83dfb",
+    },
+    "13.7.1": {
+        "darwin-x64": "e092c0a39d014b51099cea7402ae28c9f3257c538ee734703a30180f23d0350b",
+        "darwin-arm64": "85b8785ffbf0ec44c80e0c7491a50eaa250a2cd3ae2ea6c43be21b1f0bdbdad1",
+        "linux-x64": "e75d27bf6646314d904a0fb9ef9055839ea7afd287d531446d52964020e6131e",
+        "linux-arm64": "7babdf570297a22d2a4759d1c1a3a3f09339b6f4977bdf58ae0ce9fb2585f6db",
+        "win32-x64": "7d67a3b1b9f2ffb18410f6aaf2ea1621b10d80ae7d4bf1b6bcac3fa8d3c28350",
+    },
+    "13.7.0": {
+        "darwin-x64": "ae47c23738c61c63f384f9176adeb36a8dbd4e9110073f6f02c8718c0dc16709",
+        "darwin-arm64": "aa8cf8635c61be42c1a5d106143aef4062e0df68fb9d5d2e9694dfd3087f7b8a",
+        "linux-x64": "43ef8b1ca2bfb162600e319bb247747843e3b1a0d32500af2eeecaab2e84462c",
+        "linux-arm64": "59afe89eaa3bba0f4d9f2becd03fbf6881892648b661bbe3bb847a137962856d",
+        "win32-x64": "2bbbb2c93c4d8a812ba867d76cf7073323ff291a9989aff07a341c2f88a4e6e4",
+    },
     "13.6.6": {
         "darwin-x64": "4b845921f35f520b8217a4ebb9106180c56de37a9db7f57f14352060c9eddca6",
         "darwin-arm64": "4c8818534bdab028a82aefc1936a552481fa0efb2a7c5c64350e9d9d1c709c85",

diff --git a/docs/defs.md b/docs/defs.md