From a52672b9c42300f53e1cfd4af5c58bbd81c95652 Mon Sep 17 00:00:00 2001 From: Daniel Muller Date: Sun, 28 Apr 2024 20:19:18 -0600 Subject: [PATCH 1/2] 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. --- cypress/defs.bzl | 125 ++++++++++++++++++++++++------- cypress/private/cypress_test.bzl | 16 +--- cypress/private/versions.bzl | 28 +++++++ docs/defs.md | 8 +- 4 files changed, 133 insertions(+), 44 deletions(-) diff --git a/cypress/defs.bzl b/cypress/defs.bzl index db694d7..862913f 100644 --- 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, **kwargs): + tags = kwargs.pop("tags", []) + if not kwargs.pop("allow_sandbox", False): + 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", + allow_sandbox = False, + 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. + allow_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, + allow_sandbox = allow_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", + allow_sandbox = False, + 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. + allow_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, + allow_sandbox = allow_sandbox, + browsers = browsers, **kwargs ) diff --git a/cypress/private/cypress_test.bzl b/cypress/private/cypress_test.bzl index f81d4ef..6189111 100644 --- a/cypress/private/cypress_test.bzl +++ b/cypress/private/cypress_test.bzl @@ -4,20 +4,10 @@ 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( - 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 index 6109b2e..fec46bb 100644 --- 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 index 93087cd..bd6c26f 100644 --- a/docs/defs.md +++ b/docs/defs.md @@ -7,7 +7,7 @@ Public API re-exports ## cypress_module_test 
-cypress_module_test(name, runner, cypress, kwargs)
+cypress_module_test(name, runner, cypress, allow_sandbox, browsers, kwargs)
cypress_module_test creates a node environment which is hooked up to the cypress toolchain. @@ -43,6 +43,8 @@ In most scenarios, it is easier to use cypress_test. But in some scenarios, you | name | The name used for this rule and output files | none | | runner | JS file to call into the cypress module api See https://docs.cypress.io/guides/guides/module-api | none | | cypress | The cypress npm package which was already linked using an API like npm_link_all_packages. | `"//:node_modules/cypress"` | +| allow_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... | `False` | +| 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 | none | @@ -51,7 +53,7 @@ In most scenarios, it is easier to use cypress_test. But in some scenarios, you ## cypress_test 
-cypress_test(name, cypress, kwargs)
+cypress_test(name, cypress, allow_sandbox, browsers, kwargs)
cypress_test runs the cypress CLI with the cypress toolchain. @@ -70,6 +72,8 @@ https://docs.cypress.io/guides/guides/command-line#What-you-ll-learn | :------------- | :------------- | :------------- | | name | The name used for this rule and output files | none | | cypress | The cypress npm package which was already linked using an API like npm_link_all_packages. | `"//:node_modules/cypress"` | +| allow_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... | `False` | +| 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 | none | From ab30a33100397edb9325ffe57ecb02cbb2330f3a Mon Sep 17 00:00:00 2001 From: Daniel Muller Date: Tue, 30 Apr 2024 10:01:23 -0600 Subject: [PATCH 2/2] Review comments addressed - Switch to `disable_sandbox` defaulting to true - Removed unused attribute from rule impl --- cypress/defs.bzl | 16 ++++++++-------- cypress/private/cypress_test.bzl | 3 --- docs/defs.md | 8 ++++---- 3 files changed, 12 insertions(+), 15 deletions(-) diff --git a/cypress/defs.bzl b/cypress/defs.bzl index 862913f..6a09a18 100644 --- a/cypress/defs.bzl +++ b/cypress/defs.bzl @@ -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, @@ -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. @@ -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. @@ -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 ) @@ -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. @@ -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. @@ -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 ) diff --git a/cypress/private/cypress_test.bzl b/cypress/private/cypress_test.bzl index 6189111..1ad972c 100644 --- a/cypress/private/cypress_test.bzl +++ b/cypress/private/cypress_test.bzl @@ -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, ), diff --git a/docs/defs.md b/docs/defs.md index bd6c26f..26654e3 100644 --- a/docs/defs.md +++ b/docs/defs.md @@ -7,7 +7,7 @@ Public API re-exports ## cypress_module_test 
-cypress_module_test(name, runner, cypress, allow_sandbox, browsers, kwargs)
+cypress_module_test(name, runner, cypress, disable_sandbox, browsers, kwargs)
cypress_module_test creates a node environment which is hooked up to the cypress toolchain. @@ -43,7 +43,7 @@ In most scenarios, it is easier to use cypress_test. But in some scenarios, you | name | The name used for this rule and output files | none | | runner | JS file to call into the cypress module api See https://docs.cypress.io/guides/guides/module-api | none | | cypress | The cypress npm package which was already linked using an API like npm_link_all_packages. | `"//:node_modules/cypress"` | -| allow_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... | `False` | +| 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... | `True` | | 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 | none | @@ -53,7 +53,7 @@ In most scenarios, it is easier to use cypress_test. But in some scenarios, you ## cypress_test 
-cypress_test(name, cypress, allow_sandbox, browsers, kwargs)
+cypress_test(name, cypress, disable_sandbox, browsers, kwargs)
cypress_test runs the cypress CLI with the cypress toolchain. @@ -72,7 +72,7 @@ https://docs.cypress.io/guides/guides/command-line#What-you-ll-learn | :------------- | :------------- | :------------- | | name | The name used for this rule and output files | none | | cypress | The cypress npm package which was already linked using an API like npm_link_all_packages. | `"//:node_modules/cypress"` | -| allow_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... | `False` | +| 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... | `True` | | 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 | none |