Address review comments

cypress/defs.bzl

@@ -13,6 +13,9 @@ _cypress_test = rule(
 )
 
 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,
@@ -28,11 +31,7 @@ def _cypress_test_macro(name, entry_point, cypress, **kwargs):
             "@aspect_rules_js//js/private:experimental_allow_unresolved_symlinks": True,
             "//conditions:default": False,
         }),
-        # 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.
-        tags = kwargs.pop("tags", []) + ["no-sandbox"],
+        tags = tags,
         **kwargs
     )
 
@@ -79,7 +78,8 @@ def cypress_module_test(name, runner, cypress = "//:node_modules/cypress", **kwa
     cypress.run({
     headless: true,
     }).then(result => {
-    if (result.status === 'failed') {
+    // Cypress only reports a status on failures
+    if (result.status) {
         process.exit(1);
     }
     })

cypress/private/cypress_test.bzl

@@ -4,6 +4,25 @@ 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(
+        doc = """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...
+        """,
+        default = False,
+    ),
     "browsers": attr.label_list(
         doc = """A sequence of labels specifying the browsers to include.
 

e2e/workspace/MODULE.bazel

@@ -10,8 +10,11 @@ bazel_dep(name = "bazel_features", version = "1.9.0", dev_dependency = True)
 npm = use_extension("@aspect_rules_js//npm:extensions.bzl", "npm", dev_dependency = True)
 npm.npm_translate_lock(
     name = "npm",
+    # Speed up installation by disabling cypress binary install. Optional.
+    lifecycle_hooks_envs = {
+        "cypress": ["CYPRESS_INSTALL_BINARY=0"],
+    },
     pnpm_lock = "//:pnpm-lock.yaml",
-    run_lifecycle_hooks = False,
     verify_node_modules_ignored = "//:.bazelignore",
 )
 use_repo(npm, "npm")

e2e/workspace/cli_test/cypress.config.ts

@@ -1,12 +1,12 @@
-import { defineConfig } from "cypress";
+import { defineConfig } from 'cypress';
 
 export default defineConfig({
   e2e: {
-    specPattern: ["cli_test.cy.ts"],
+    specPattern: ['cli_test.cy.ts'],
     supportFile: false,
     setupNodeEvents(on, config) {
-      on("before:browser:launch", (browser, launchOptions) => {
-        launchOptions.args.push("--disable-gpu-shader-disk-cache");
+      on('before:browser:launch', (browser, launchOptions) => {
+        launchOptions.args.push('--disable-gpu-shader-disk-cache');
       });
     },
   },

e2e/workspace/module_test/runner.js

@@ -12,6 +12,7 @@ async function main() {
     return 1;
   }
 
+  // Cypress only reports a status on failures
   if (result.status) {
     console.error("Cypress tests failed with status", result.status);
     return 2;