From 513e610eeeb3301055756e3b87ea09c5a7fd7bf5 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 16 Jan 2026 18:54:42 +0400 Subject: [PATCH 01/11] Experiments with running playwright tests in bazel --- app/electron-client/BUILD.bazel | 19 + .../PLAYWRIGHT_BAZEL_INVESTIGATION.md | 851 ++++++++++++++++++ app/electron-client/playwright-wrapper.mjs | 57 ++ .../selective-symlink-loader-hooks.mjs | 62 ++ .../selective-symlink-loader.mjs | 23 + .../tests/cloudWorkflow.spec.ts | 6 +- app/electron-client/tests/electronTest.ts | 79 +- .../tests/gettingStarted.spec.ts | 6 +- .../tests/headless/package.json | 3 + .../tests/headless/writeFile.test.ts | 7 +- .../tests/localWorkflow.spec.ts | 6 +- app/electron-client/tests/package.json | 3 + app/gui/tsconfig.app.json | 4 +- 13 files changed, 1080 insertions(+), 46 deletions(-) create mode 100644 app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md create mode 100644 app/electron-client/playwright-wrapper.mjs create mode 100644 app/electron-client/selective-symlink-loader-hooks.mjs create mode 100644 app/electron-client/selective-symlink-loader.mjs create mode 100644 app/electron-client/tests/headless/package.json create mode 100644 app/electron-client/tests/package.json diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index 4c7cf0a8e582..ae5b5b62892b 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -1,9 +1,11 @@ load("@aspect_bazel_lib//lib:write_source_files.bzl", "write_source_files") load("@aspect_rules_esbuild//esbuild:defs.bzl", "esbuild") +load("@aspect_rules_js//js:defs.bzl", "js_test") load("@aspect_rules_js//npm:defs.bzl", "npm_package") load("@aspect_rules_ts//ts:defs.bzl", "ts_config", "ts_project") load("@npm//:defs.bzl", "npm_link_all_packages", "npm_link_targets") load("@npm//app/electron-client:electron-builder/package_json.bzl", electron_builder_bin = "bin") +load("@npm//app/electron-client:playwright/package_json.bzl", playwright_bin = "bin") load("//:bazel_scripts/ts_config.bzl", "write_tsconfig") load("//internal:stampFiles.bzl", "stamp_files") @@ -166,3 +168,20 @@ write_source_files( suggested_update_target = "//:write_all", visibility = ["//visibility:public"], ) + +js_test( + name = "test", + entry_point = "playwright-wrapper.mjs", + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "package.json", + "playwright.config.ts", + "tests/package.json", + "selective-symlink-loader.mjs", + "selective-symlink-loader-hooks.mjs", + ], + args = ["test"], + chdir = package_name(), + visibility = ["//visibility:public"], + log_level = "debug", + tags = ["local"], +) diff --git a/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md b/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md new file mode 100644 index 000000000000..6632616212af --- /dev/null +++ b/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md @@ -0,0 +1,851 @@ +# Playwright + Bazel Integration Investigation + +## Problem Statement +`bazel test //app/electron-client:test` fails, while `pnpm run ide-integration-test` works. + +**Error:** +``` +Error: Playwright Test did not expect test() to be called here. +Most common reasons include: +- You are calling test() in a configuration file. +- You are calling test() in a file that is imported by the configuration file. +- You have two different versions of @playwright/test. +``` + +## Root Cause Analysis + +### Trigger Condition (Confirmed) +The `"type": "module"` field in package.json triggers the issue. Controlled experiments with `rules_playwright/examples/rules_js` (a separate repo used only for testing, not part of electron-client) confirmed: + +| Change to baseline (no `"type": "module"`) | Result | +|--------------------------------------------|--------| +| Add fixture wrapper file | ✅ PASSES | +| Add `"type": "module"` alone | ❌ FAILS (dual instance error) | +| Fixture wrapper + `"type": "module"` | ❌ FAILS | + +**Note:** electron-client uses the generated `playwright_bin` from `@npm//app/electron-client:playwright/package_json.bzl` directly. It does not use `rules_playwright`. + +### Initial Theory: `--preserve-symlinks-main` (DISPROVEN) + +We initially believed the issue was `--preserve-symlinks-main` being passed by aspect_rules_js. However, **debug logs show this flag is NOT being passed**: + +``` +DEBUG: aspect_rules_js[js_test]: JS_BINARY__NODE_OPTIONS +INFO: aspect_rules_js[js_test]: running .../test_node_bin/node -- .../playwright/cli.js test +``` + +The `--` separator comes immediately after the node path, meaning no node flags are added. Setting `preserve_symlinks_main = False` on the rule was tried and **did not fix the issue**. + +### Previous Theory: Node FS Patches (DISPROVEN) + +The debug logs reveal that aspect_rules_js applies **filesystem patches** to Node.js: + +``` +DEBUG: aspect_rules_js[js_test]: JS_BINARY__NODE_PATCHES .../aspect_rules_js+/js/private/node-patches/register.cjs +DEBUG: aspect_rules_js[js_test]: node fs patches will be applied with roots: ... +``` + +The node wrapper script (`test_node_bin/node`) does: +```bash +exec "$JS_BINARY__NODE_BINARY" --require "$JS_BINARY__NODE_PATCHES" "$@" +``` + +**We initially theorized** that these patches intercept `realpath`, `lstat`, `readlink`, and other fs operations, causing ESM module identity issues. However, **debug logging disproved this theory.** + +### Debug Logging Results (2026-01-21) + +Added comprehensive debug logging across config, fixture, and test files: + +**Module Resolution URLs (all identical):** +``` +[playwright.config] playwright/test resolved to: file://.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.mjs +[electronTest] playwright/test resolved to: file://.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.mjs +[cloudWorkflow.spec] playwright/test resolved to: file://.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.mjs +``` + +**Object Identity Check:** +``` +[cloudWorkflow.spec] base === globalThis.__playwrightTestFromConfig: true +``` + +**Critical Finding: There is NO dual module instance.** The `test` object is the SAME instance across config, fixture, and test files. The error message is misleading. + +### Previous Theory: Suite Context Not Initialized (SUPERSEDED) + +The error "Playwright Test did not expect test() to be called here" was initially thought to be about suite context. However, the CommonJS experiment revealed the TRUE root cause. + +### Previous Theory: Sandbox Path Mismatch (PARTIALLY CORRECT) + +Initial analysis suggested sandbox paths (`sandbox/processwrapper-sandbox/N/`) caused the issue. However, testing with `tags = ["local"]` and `--strategy=TestRunner=local,standalone` revealed the sandbox is NOT the root cause. + +### Current Theory: Dual node_modules Locations (2026-01-21) + +Running with local execution (no sandbox) still shows the dual-module error. The paths reveal the TRUE issue: + +**First load:** `bin/node_modules/.aspect_rules_js/playwright@1.55.1/...` +**Second load:** `bin/app/electron-client/test_/test.runfiles/_main/node_modules/.aspect_rules_js/playwright@1.55.1/...` + +Bazel creates TWO copies of node_modules: +1. **Direct:** `bazel-out/darwin_arm64-opt/bin/node_modules/` +2. **Runfiles:** `bazel-out/darwin_arm64-opt/bin/app/electron-client/test_/test.runfiles/_main/node_modules/` + +When Playwright loads: +1. Config file resolution uses the direct `bin/node_modules/` path +2. Test file resolution uses the runfiles `test.runfiles/_main/node_modules/` path + +Node.js sees these as two different modules (different paths = different cache entries), causing Playwright's dual-instance detection to trigger. + +### The Dilemma (Revised) + +| Approach | Result | +|----------|--------| +| Default (sandbox) | sandbox path + runfiles path mismatch | +| Local execution | bin/node_modules + runfiles/node_modules mismatch | +| FS patches OFF | "No tests found" | + +The issue is fundamental to how aspect_rules_js sets up the runfiles tree with a separate copy of node_modules. + +## Why electron-client Requires ESM +- Codebase uses ES module syntax throughout +- `electronTest.ts` uses `import.meta.dirname` +- Many dependencies expect ESM +- Removing `"type": "module"` would require significant refactoring + +## What Was Tried (All Failed) + +| Approach | Result | Why It Failed | +|----------|--------|---------------| +| `preserve_symlinks_main = False` | Same error | Flag wasn't the issue (not being passed anyway) | +| Remove top-level await | Same error | Not the cause | +| Direct import pattern | Same error | Module identity issue is at fs level, not import level | +| Custom test runner (clear `JS_BINARY__*` env vars) | "No tests found" | Module resolution breaks without Bazel wrapper | +| `NODE_OPTIONS="--no-preserve-symlinks"` | No effect | Flag wasn't being passed via NODE_OPTIONS | +| `JS_BINARY__PATCH_NODE_FS=0` | "No tests found" | Playwright can't traverse symlinks to find tests | +| Force CommonJS for tests | Different error | Dual-module issue persists due to path mismatch (see below) | +| `tags = ["local"]` + `--strategy=TestRunner=local` | Same error | Not sandbox-related; bin/node_modules vs runfiles/node_modules | +| `NODE_PATH` via env (relative path) | Same error | NODE_PATH doesn't override normal resolution; config symlink still resolves to bin/ | +| `NODE_PATH` via env (`$$JS_BINARY__RUNFILES`) | Script error | Variable not defined when env vars are exported (line 15 vs line 273) | +| `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` | Different error | Breaks pnpm-style nested symlinks; `Cannot find module 'playwright-core'` | + +### CommonJS Approach Results (2026-01-21) + +Created `tests/package.json` with `{"type": "commonjs"}` and replaced `import.meta.dirname` with `__dirname` in `electronTest.ts`. + +**Result:** Different error that reveals the TRUE dual-module issue: + +``` +Error: Requiring @playwright/test second time, +First: + at cloudWorkflow.spec.ts:3 +> 3 | import { test as base, expect } from 'playwright/test' + at Object. (.../execroot/_main/bazel-out/.../playwright/lib/index.js:60:33) + ... + +Second: + at Object. (.../execroot/_main/bazel-out/.../playwright/lib/index.js:60:33) + at Object. (.../sandbox/processwrapper-sandbox/5/execroot/_main/.../tests/cloudWorkflow.spec.ts:3:1) +``` + +**Key Insight:** The paths reveal why dual-module occurs: +- **Config loading:** Uses `execroot/_main/bazel-out/...` +- **Test file loading:** Uses `sandbox/processwrapper-sandbox/5/execroot/_main/bazel-out/...` + +Node.js CommonJS module caching is path-based. When the same physical module is accessed through different paths (execroot vs sandbox), Node treats them as separate modules. Playwright detects this and throws an error. + +This confirms the issue is NOT about ESM vs CommonJS loaders - it's fundamentally about **Bazel's sandbox creating distinct path prefixes** that break Node.js module identity. + +### NODE_PATH Experiment (2026-01-21) + +Attempted to use `NODE_PATH` environment variable to force consistent module resolution. + +**Attempt 1: Bazel variable expansion** +```python +env = { + "NODE_PATH": "$$JS_BINARY__RUNFILES/_main/node_modules", +}, +``` + +**Result:** Script error - `JS_BINARY__RUNFILES: unbound variable` + +**Why it failed:** The generated shell script has `set -o nounset` and exports env vars from the `env` parameter on line 15, but `JS_BINARY__RUNFILES` isn't computed until line 273. The Bazel `$$` correctly expands to `$` at runtime, but the variable doesn't exist yet when the export happens. + +**Attempt 2: Relative path** +```python +env = { + "NODE_PATH": "../../node_modules", +}, +``` + +**Result:** Same dual-module error. + +**Why it failed:** NODE_PATH is only consulted AFTER normal module resolution fails. When a file imports `@playwright/test`, Node.js first: +1. Resolves the importing file's path (following symlinks) +2. Walks up from that resolved location looking for `node_modules/` +3. Only if that fails, checks NODE_PATH + +Since `playwright.config.ts` is a symlink to `bin/app/electron-client/playwright.config.ts`, Node resolves `@playwright/test` from `bin/node_modules/` before ever checking NODE_PATH. + +### --preserve-symlinks Experiment (2026-01-21) + +Attempted to prevent Node from resolving symlinks when determining file locations: + +```python +env = { + "NODE_OPTIONS": "--preserve-symlinks --preserve-symlinks-main", +}, +``` + +**Result:** Different error - `Cannot find module 'playwright-core'` + +**Partial success:** The dual-module error was eliminated! With `--preserve-symlinks`, the config file's location is treated as `runfiles/_main/app/electron-client/playwright.config.ts` (the symlink path) rather than `bin/app/electron-client/playwright.config.ts` (the target path). + +**Why it ultimately failed:** The `--preserve-symlinks` flag affects ALL symlinks, including those used by pnpm/aspect_rules_js for the nested node_modules structure: + +``` +app/electron-client/node_modules/playwright -> ../../../node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright +``` + +When Node doesn't follow this symlink, it can't find `playwright-core` which is a peer dependency located at: +``` +node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright-core +``` + +**Key insight:** We need SELECTIVE symlink preservation - preserve symlinks for source files (config, tests) but follow symlinks for node_modules. Node.js doesn't support this granularity. + +### Symlink Structure Analysis (2026-01-21) + +Verified that both config and test files are symlinks to `bin/`: + +``` +runfiles/_main/app/electron-client/playwright.config.ts + -> bin/app/electron-client/playwright.config.ts + +runfiles/_main/app/electron-client/tests/cloudWorkflow.spec.ts + -> bin/app/electron-client/tests/cloudWorkflow.spec.ts +``` + +Local node_modules in runfiles uses relative symlinks to the pnpm store: +``` +runfiles/_main/app/electron-client/node_modules/playwright + -> ../../../node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright +``` + +## Potential Solutions (Revised) + +### ~~1. Debug Module Resolution Paths~~ ✅ Done +CommonJS error output revealed the exact paths causing dual-module issues. + +### ~~2. Force CommonJS for Tests~~ ❌ Failed +Tried `tests/package.json` with `{"type": "commonjs"}`. Same underlying issue - path mismatch causes dual-module. + +### ~~3. Disable Bazel Sandboxing~~ ❌ Failed +Tried `tags = ["local"]` + `--strategy=TestRunner=local,standalone`. Issue is NOT sandboxing - it's bin/node_modules vs runfiles/node_modules. + +### ~~4. Ensure Consistent node_modules Resolution via NODE_PATH~~ ❌ Failed +Tried setting `NODE_PATH` via env parameter. Two issues: +1. Using `$$JS_BINARY__RUNFILES` fails because the variable isn't defined when env vars are exported +2. Using relative paths doesn't help because NODE_PATH is only consulted after normal resolution fails + +### ~~5. Use --preserve-symlinks~~ ❌ Partial Success, Ultimately Failed +Setting `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` fixes the dual-module error but breaks pnpm-style nested symlinks in node_modules, causing `Cannot find module 'playwright-core'`. + +**Key insight:** We need selective symlink preservation - preserve for source files, follow for node_modules. Node.js doesn't support this. + +### 6. Ensure Consistent node_modules Resolution (Revised) +The core problem remains: Playwright resolves `playwright/test` from two locations: +- `bin/node_modules/` (when loading config, after symlink resolution) +- `test.runfiles/_main/node_modules/` (when loading test files) + +Remaining approaches to try: +- Copy files instead of symlinking (if aspect_rules_js supports this) +- Use a custom Node.js loader that selectively handles symlinks +- Patch the generated shell script to set NODE_PATH after JS_BINARY__RUNFILES is computed + +### 7. Pre-compile TypeScript +Compile tests to JavaScript first. This is [officially documented by Playwright](https://playwright.dev/docs/test-typescript). + +**Blocker:** TypeScript errors in test files need fixing first. + +**Note:** Bundling test files with esbuild is NOT a proven approach. The Playwright team stated ["Playwright isn't ready to be bundled"](https://github.com/microsoft/playwright/issues/35479). + +### 8. File Issue with aspect_rules_js +The fundamental issue is that aspect_rules_js creates two node_modules locations: +- Direct: `bin/node_modules/` +- Runfiles: `test.runfiles/_main/node_modules/` + +When a tool like Playwright resolves the same package from both locations, Node.js treats them as separate modules. This may require upstream fixes. + +### 9. Investigate `include_npm_sources` or Similar Options +Check if aspect_rules_js has options to control how node_modules are linked/copied into the runfiles tree. + +## Current BUILD.bazel State +```python +playwright_bin.playwright_test( + name = "test", + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "package.json", + "playwright.config.ts", + "tests/package.json", # Added for CommonJS experiment + ], + args = ["test"], + chdir = package_name(), + visibility = ["//visibility:public"], + preserve_symlinks_main = False, + log_level = "debug", + tags = ["local"], # Tried to bypass sandboxing - didn't fix issue +) +``` + +**Note:** Requires `--strategy=TestRunner=local,standalone` flag when running. + +## Current Test Files State +- `tests/package.json` - Contains `{"type": "commonjs"}` (for CommonJS experiment) +- `tests/headless/package.json` - Contains `{"type": "module"}` (preserves ESM for Vitest) +- `tests/electronTest.ts` - Uses `__dirname` instead of `import.meta.dirname` + +## Key Files +- `app/electron-client/BUILD.bazel` - Bazel config +- `app/electron-client/tests/electronTest.ts` - Test fixtures +- `app/electron-client/playwright.config.ts` - Playwright config +- `test_node_bin/node` - Generated node wrapper that applies fs patches +- `aspect_rules_js+/js/private/node-patches/register.cjs` - FS patch registration +- `aspect_rules_js+/js/private/node-patches/fs.cjs` - FS patch implementation + +## Debug Logs Reference + +Enable debug logs with: +```bash +JS_BINARY__LOG_DEBUG=1 bazel test //app/electron-client:test +``` + +Key environment variables: +- `JS_BINARY__NODE_BINARY` - Path to actual Node.js binary +- `JS_BINARY__NODE_WRAPPER` - Path to wrapper script +- `JS_BINARY__NODE_PATCHES` - Path to fs patches module +- `JS_BINARY__PATCH_NODE_FS` - Set to "0" to disable fs patches +- `JS_BINARY__FS_PATCH_ROOTS` - Allowed roots for fs operations + +## Next Steps + +### Completed +1. ~~**Add debug logging** to trace module resolution paths~~ ✅ Done - URLs and instances are identical +2. ~~**Try `--workers=1`**~~ ✅ Done - no effect (config already sets `workers: 1`) +3. ~~**Verify module instance identity**~~ ✅ Done - confirmed same instance via globalThis check +4. ~~**Try adding `tests/package.json` with `"type": "commonjs"`**~~ ✅ Done - revealed path mismatch issue +5. ~~**Try `tags = ["local"]`**~~ ✅ Done - confirmed issue is NOT sandboxing, but bin vs runfiles node_modules +6. ~~**Try setting `NODE_PATH`**~~ ✅ Done - doesn't work (consulted after normal resolution) +7. ~~**Try `--preserve-symlinks`**~~ ✅ Done - partial success, breaks pnpm nested symlinks + +### Remaining +8. **Investigate aspect_rules_js node_modules linking** - understand why two locations exist and if it can be configured +9. **File issue with aspect_rules_js** - report bin/node_modules vs runfiles/node_modules incompatibility with Playwright +10. **Check if there's a `copy_data_to_bin` option** - or similar to control whether files are symlinked or copied +11. **Try custom Node.js loader** - selectively handle symlinks for source files vs node_modules +12. **Investigate patching the generated shell script** - set NODE_PATH after JS_BINARY__RUNFILES is computed +13. **Pre-compile TypeScript** - may avoid the issue by not using Playwright's TS transform + +--- + +## Wrapper Script Approach (2026-01-21) + +### Approach +Created a custom JavaScript wrapper (`playwright-wrapper.mjs`) that: +1. Runs after Bazel sets up `JS_BINARY__RUNFILES` +2. Sets `NODE_PATH` at runtime to force consistent module resolution +3. Spawns Playwright CLI with the corrected environment + +### Implementation + +**`playwright-wrapper.mjs`:** +```javascript +#!/usr/bin/env node +import { spawn } from 'node:child_process'; +import path from 'node:path'; + +const runfilesDir = process.env.JS_BINARY__RUNFILES; +if (!runfilesDir) { + console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.'); + process.exit(1); +} + +const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules'); +const newNodePath = process.env.NODE_PATH + ? `${nodeModulesPath}:${process.env.NODE_PATH}` + : nodeModulesPath; + +const playwrightCli = path.join( + nodeModulesPath, + '.aspect_rules_js', + 'playwright@1.55.1', + 'node_modules', + 'playwright', + 'cli.js' +); + +const child = spawn( + process.execPath, + [playwrightCli, ...process.argv.slice(2)], + { + stdio: 'inherit', + env: { ...process.env, NODE_PATH: newNodePath }, + cwd: process.cwd(), + } +); + +child.on('exit', (code, signal) => { + if (signal) process.kill(process.pid, signal); + else process.exit(code ?? 1); +}); +``` + +**BUILD.bazel change:** +Replaced `playwright_bin.playwright_test` with `js_test`: +```python +load("@aspect_rules_js//js:defs.bzl", "js_test") + +js_test( + name = "test", + entry_point = "playwright-wrapper.mjs", + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "package.json", + "playwright.config.ts", + "tests/package.json", + ], + args = ["test"], + chdir = package_name(), + visibility = ["//visibility:public"], + log_level = "debug", + tags = ["local"], +) +``` + +### Known Limitations +1. **Playwright version hardcoded**: The path includes `playwright@1.55.1`. If version changes, wrapper needs update. +2. **NODE_PATH priority**: NODE_PATH is consulted after normal resolution. If symlinks still resolve first, may need to combine with `--preserve-symlinks`. +3. **Windows paths**: Uses `:` as PATH separator. Use `path.delimiter` if Windows support needed. + +### Result +**FAILED** - Same dual-module error persists. + +**Error output:** +``` +Error: Requiring @playwright/test second time, +First: + at Object. (.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/lib/index.js:60:33) + at Object. (.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.js:17:13) + at Object. (.../test.runfiles/_main/app/electron-client/tests/cloudWorkflow.spec.ts:3:1) + +Second: + at Object. (.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/lib/index.js:60:33) + at Object. (.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.js:17:13) + at Object. (.../test.runfiles/_main/app/electron-client/tests/cloudWorkflow.spec.ts:3:1) +``` + +**Why it failed:** +Setting `NODE_PATH` in the spawned child process doesn't prevent normal module resolution. Node.js module resolution order is: +1. Normal resolution (walk up from file location to find `node_modules/`) +2. Only if step 1 fails, consult `NODE_PATH` + +The config file `playwright.config.ts` is a symlink pointing to `bin/app/electron-client/playwright.config.ts`. When Playwright loads it and resolves `playwright/test`: +1. Node resolves the symlink to `bin/...` location +2. Node walks up from `bin/app/electron-client/` and finds `bin/node_modules/` +3. `NODE_PATH` is never consulted because step 2 succeeded + +The test files also resolve `playwright/test`: +1. Test file is in `runfiles/_main/app/electron-client/tests/` +2. Node walks up and finds `runfiles/_main/node_modules/` + +Result: Same module, two paths, dual-instance error. + +**Conclusion:** +The wrapper script approach cannot solve the dual-module issue because `NODE_PATH` doesn't override normal module resolution - it's a fallback, not a priority override. + +--- + +## Custom ESM Loader Approach (2026-01-21) + +### Approach +Created a custom ESM loader that selectively rewrites module resolution paths: +- **Preserves symlinks for source files** - config and tests stay in `runfiles/_main/` +- **Follows symlinks for node_modules** - pnpm's nested structure works normally + +This approach uses the `--import` flag to register a custom `resolve` hook before Playwright runs. + +### Implementation + +**`selective-symlink-loader.mjs`:** +```javascript +// Rewrite bin/ paths back to runfiles for source files (not node_modules) +export async function resolve(specifier, context, nextResolve) { + const result = await nextResolve(specifier, context); + + // Only modify source files, not node_modules + if ( + result.url && + result.url.includes('/bin/') && + !result.url.includes('node_modules') + ) { + // Rewrite: bin/app/electron-client/... -> runfiles/_main/app/electron-client/... + const rewritten = result.url.replace( + /\/bin\/(app\/electron-client\/)/, + '/test.runfiles/_main/$1' + ); + if (rewritten !== result.url) { + return { ...result, url: rewritten }; + } + } + + return result; +} +``` + +**Updated `playwright-wrapper.mjs`:** +```javascript +const loaderPath = path.join( + runfilesDir, + '_main', + 'app', + 'electron-client', + 'selective-symlink-loader.mjs' +); + +const child = spawn( + process.execPath, + ['--import', loaderPath, playwrightCli, ...process.argv.slice(2)], + { /* ... */ } +); +``` + +**Updated BUILD.bazel:** +```python +js_test( + name = "test", + entry_point = "playwright-wrapper.mjs", + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "package.json", + "playwright.config.ts", + "tests/package.json", + "selective-symlink-loader.mjs", # Added loader + ], + # ... +) +``` + +### Verification +```bash +bazel test //app/electron-client:test --strategy=TestRunner=local,standalone +``` + +### Result +**SUCCESS** - The dual-module error is resolved! + +The initial ESM-only approach (using `export async function resolve`) didn't work because: +1. ESM hooks exported from a module loaded via `--import` aren't automatically registered +2. Playwright uses CommonJS internally (via pirates) for TypeScript transformation + +The solution required: +1. Use `module.register()` to properly register ESM hooks from a separate file +2. Patch both CJS (`Module._resolveFilename`) and ESM (`resolve` hook) paths +3. Rewrite `bin/node_modules/` paths to `runfiles/_main/node_modules/` using the `JS_BINARY__RUNFILES` env var + +**Final Implementation:** +- `selective-symlink-loader.mjs` - Main loader that patches CJS and registers ESM hooks +- `selective-symlink-loader-hooks.mjs` - ESM hooks file registered via `module.register()` + +### New Error (Unrelated) +After fixing the dual-module issue, tests now fail with: +``` +Error: Package subpath './src/text' is not defined by "exports" in .../node_modules/enso-common/package.json +``` + +This is a **separate issue** - the `electronTest.ts` file imports `enso-common/src/text` which isn't exported from the package. This needs to be fixed by either: +1. Adding `./src/text` to `enso-common`'s `exports` field +2. Changing the import in `electronTest.ts` to use an exported path + +### Files Created/Modified +- `selective-symlink-loader.mjs` - Custom module loader (CJS hook + ESM registration) +- `selective-symlink-loader-hooks.mjs` - ESM loader hooks +- `playwright-wrapper.mjs` - Updated to use `--import` flag with the loader +- `BUILD.bazel` - Added loader files to `data` dependencies + +--- + +## `enso-common/src/text` Import Error Investigation (2026-01-21) + +### Problem +After fixing the dual-module issue, tests fail with: +``` +Error: Package subpath './src/text' is not defined by "exports" in .../node_modules/enso-common/package.json +``` + +### Initial Plan: Use `--conditions=source` Flag +The plan was to add `--conditions=source` to Node.js arguments in `playwright-wrapper.mjs`. The theory was: +1. `enso-common/package.json` has exports with a `"source"` condition that points to TypeScript files +2. Adding `--conditions=source` would make Node.js use this condition +3. Playwright already transforms TypeScript on-the-fly + +### Result of `--conditions=source` Approach +**FAILED** - Error changed to: +``` +Error: Cannot find module '.../enso-common/src/text.ts' +``` + +The `--conditions=source` flag DID work for exports resolution (Node.js now tried to load `.ts` files), but the source files don't exist in the Bazel sandbox. + +### Key Findings + +#### 1. The Bazel Sandbox Structure +The `enso-common` npm_package only includes: +- `package.json` +- `dist/*.js` (compiled TypeScript output) + +It does NOT include: +- `src/*.ts` (TypeScript source files) + +This is because the `npm_package` rule in `app/common/BUILD.bazel` only includes `:tsc` (compiled output), not the source files. + +#### 2. Two Resolution Paths Verified +**Direct test outside Bazel (works):** +```bash +cd app/electron-client && node -e "import('enso-common/src/text')" +# Resolves to: .../enso-common/dist/text.js (correct!) +``` + +**Inside Bazel via Playwright (fails):** +``` +Error: Package subpath './src/text' is not defined by "exports" +``` + +#### 3. CJS vs ESM Resolution Difference +Debug logging revealed the critical issue: +``` +[DEBUG] CJS resolving: "enso-common/src/text" from: .../tests/electronTest.ts +[DEBUG] CJS resolve FAILED for "enso-common/src/text": Package subpath './src/text' is not defined by "exports" +``` + +The resolution is happening via **CommonJS `Module._resolveFilename`**, NOT ESM `import()`. This is because Playwright uses CommonJS internally for TypeScript transformation (via pirates). + +#### 4. The Root Cause: Missing "require" Export Condition +The `enso-common/package.json` exports field: +```json +{ + "./src/*": { + "source": "./src/*.ts", + "types": "./src/*.ts", + "import": "./dist/*.js" + } +} +``` + +**Problem:** There is NO `"require"` condition! + +- ESM `import()` uses the `"import"` condition → `./dist/*.js` ✅ +- CJS `require()` uses the `"require"` condition → **not defined** ❌ + +Node.js CJS resolution for exports patterns requires a `"require"` condition. Without it, the pattern matching fails even though the pattern syntax is correct. + +#### 5. Why Direct Test Works +When testing directly with `node -e "import('enso-common/src/text')"`: +- Uses ESM `import()` semantics +- Uses the `"import"` condition +- Pattern `./src/*` matches `./src/text` → resolves to `./dist/text.js` + +When Playwright loads via CJS: +- Uses CJS `require()` semantics +- Looks for `"require"` condition +- No `"require"` condition defined → fails with "subpath not defined" + +### Verified Facts +1. ✅ The `dist/text.js` file EXISTS in the Bazel sandbox +2. ✅ The `package.json` with correct exports IS present in sandbox +3. ✅ The wildcard pattern `./src/*` is syntactically correct +4. ✅ ESM resolution works correctly (verified with direct `node -e "import()"` test) +5. ❌ CJS resolution fails due to missing `"require"` condition + +### Solutions to Explore + +#### Option A: Add "require" Condition to Workspace Packages +Update `app/common/package.json` to include a `"require"` condition: +```json +{ + "./src/*": { + "source": "./src/*.ts", + "types": "./src/*.ts", + "import": "./dist/*.js", + "require": "./dist/*.js" + } +} +``` + +**Pros:** +- Simple fix +- Standard Node.js pattern +- Works for all workspace packages + +**Cons:** +- Need to update ALL workspace packages (enso-common, ydoc-shared, etc.) +- May need to verify CJS compatibility of compiled output + +#### Option B: Change Import Syntax in Test Files +Change `electronTest.ts` from: +```typescript +import { TEXTS } from 'enso-common/src/text' +``` +To: +```typescript +import { TEXTS } from 'enso-common' // If TEXTS is re-exported from index +``` +Or use dynamic import: +```typescript +const { TEXTS } = await import('enso-common/src/text') +``` + +**Pros:** +- No package.json changes needed + +**Cons:** +- Requires TEXTS to be exported from main entry point +- Dynamic import changes code structure + +#### Option C: Include Source Files in npm_package +Modify `app/common/BUILD.bazel` to include `src/*.ts` in the npm_package: +```python +npm_package( + name = "pkg", + srcs = [ + "package.json", + ":tsc", + ] + glob(["src/**/*.ts"]), # Add source files + visibility = ["//visibility:public"], +) +``` + +Then `--conditions=source` would work. + +**Pros:** +- `--conditions=source` approach becomes viable +- Source maps would work better + +**Cons:** +- Increases package size +- Need to update all workspace packages +- May cause confusion about which files are used + +#### Option D: Force ESM for Test Files +Investigate why Playwright uses CJS for test file transformation. Options: +- Configure Playwright to use ESM loader for TypeScript +- Use a different TypeScript transformer that preserves ESM + +**Pros:** +- Fixes root cause of CJS/ESM mismatch + +**Cons:** +- May require Playwright configuration not documented +- Playwright's TS handling is internal implementation detail + +### Recommended Next Step +**Option A (Add "require" condition)** is the most straightforward fix: + +1. Update `app/common/package.json`: + ```json + { + "./src/*": { + "source": "./src/*.ts", + "types": "./src/*.ts", + "import": "./dist/*.js", + "require": "./dist/*.js" + } + } + ``` + +2. Verify with test: + ```bash + bazel test //app/electron-client:test --strategy=TestRunner=local,standalone + ``` + +3. If successful, update other workspace packages that may have similar imports: + - `ydoc-shared` + - Other packages with `./src/*` exports + +### Current State +- `playwright-wrapper.mjs` does NOT have `--conditions=source` (reverted) +- Debug logging in `selective-symlink-loader.mjs` has been removed +- Tests still fail with "Package subpath not defined" error + +--- + +## SOLUTION FOUND (2026-01-21) + +### Root Cause Identified + +The `tests/package.json` file had `"type": "commonjs"` leftover from a previous debugging experiment. This was forcing ALL test files to use CommonJS resolution, which: + +1. Used the `"require"` condition instead of `"import"` for exports +2. The workspace packages (`enso-common`, etc.) only had `"import"` condition +3. Result: "Package subpath not defined" error + +### The Fix + +**Two simple changes:** + +1. **`tests/package.json`**: Changed from `"type": "commonjs"` to `"type": "module"` +2. **`tests/electronTest.ts`**: Changed `__dirname` back to `import.meta.dirname` (was also changed during the CommonJS experiment) + +### Why It Works Now + +With `"type": "module"` in `tests/package.json`: +- Playwright uses ESM resolution for test files +- ESM resolution uses the `"import"` condition from exports +- `enso-common/src/text` correctly resolves to `enso-common/dist/text.js` + +### Simplified Loader + +The CJS hook in `selective-symlink-loader.mjs` was removed since it's not needed for ESM projects. The loader now only registers ESM hooks via `module.register()`. + +### Final Files + +**`tests/package.json`:** +```json +{ + "type": "module" +} +``` + +**`selective-symlink-loader.mjs`:** +```javascript +import { register } from 'node:module'; +register('./selective-symlink-loader-hooks.mjs', import.meta.url); +``` + +**`selective-symlink-loader-hooks.mjs`:** (unchanged - rewrites bin/node_modules to runfiles/node_modules for ESM) + +**BUILD.bazel:** +```python +js_test( + name = "test", + entry_point = "playwright-wrapper.mjs", + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "package.json", + "playwright.config.ts", + "tests/package.json", + "selective-symlink-loader.mjs", + "selective-symlink-loader-hooks.mjs", + ], + args = ["test"], + chdir = package_name(), + visibility = ["//visibility:public"], + log_level = "debug", + tags = ["local"], +) +``` + +### Test Results + +Tests now run successfully (no module resolution errors). They fail with "Cannot find Enso package executable" which is expected - the Electron app needs to be built separately. The Playwright + Bazel integration issue is **RESOLVED**. + +### Key Learnings + +1. **Always check for leftover debugging changes** - the `tests/package.json` with `"type": "commonjs"` was from an earlier experiment and was never reverted +2. **ESM vs CJS resolution differs significantly** - ESM uses `"import"` condition, CJS uses `"require"` condition +3. **The custom ESM loader IS still needed** - it handles Playwright's dual-module detection by ensuring consistent module paths +4. **No changes to workspace packages required** - the fix was entirely in the test configuration diff --git a/app/electron-client/playwright-wrapper.mjs b/app/electron-client/playwright-wrapper.mjs new file mode 100644 index 000000000000..e37d3526a40c --- /dev/null +++ b/app/electron-client/playwright-wrapper.mjs @@ -0,0 +1,57 @@ +#!/usr/bin/env node +/** + * Custom Playwright test runner wrapper for Bazel. + * Uses a custom ESM loader to solve dual-module instance issues. + * + * Problem: Bazel creates two node_modules paths (bin/ and runfiles/). + * Solution: Use --import flag to load a custom ESM resolver that rewrites + * bin/ paths back to runfiles/ for source files. + */ +import { spawn } from 'node:child_process'; +import path from 'node:path'; + +const runfilesDir = process.env.JS_BINARY__RUNFILES; +if (!runfilesDir) { + console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.'); + process.exit(1); +} + +// Set NODE_PATH to runfiles node_modules +const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules'); +const newNodePath = process.env.NODE_PATH + ? `${nodeModulesPath}:${process.env.NODE_PATH}` + : nodeModulesPath; + +// Path to our custom ESM loader +const loaderPath = path.join( + runfilesDir, + '_main', + 'app', + 'electron-client', + 'selective-symlink-loader.mjs' +); + +// Playwright CLI path in runfiles +const playwrightCli = path.join( + nodeModulesPath, + '.aspect_rules_js', + 'playwright@1.55.1', + 'node_modules', + 'playwright', + 'cli.js' +); + +const child = spawn( + process.execPath, + ['--import', loaderPath, playwrightCli, ...process.argv.slice(2)], + { + stdio: 'inherit', + env: { ...process.env, NODE_PATH: newNodePath }, + cwd: process.cwd(), + } +); + +child.on('exit', (code, signal) => { + if (signal) process.kill(process.pid, signal); + else process.exit(code ?? 1); +}); diff --git a/app/electron-client/selective-symlink-loader-hooks.mjs b/app/electron-client/selective-symlink-loader-hooks.mjs new file mode 100644 index 000000000000..47475eae0e25 --- /dev/null +++ b/app/electron-client/selective-symlink-loader-hooks.mjs @@ -0,0 +1,62 @@ +/** + * ESM loader hooks for Bazel + Playwright integration. + * This file is registered via module.register() from selective-symlink-loader.mjs + */ + +import { fileURLToPath, pathToFileURL } from 'node:url'; +import fs from 'node:fs'; +import path from 'node:path'; + +// Get the runfiles node_modules path from the main loader +// This is set by the main loader before registering this hooks file +let runfilesNodeModules = null; + +export function initialize(data) { + // Get runfiles path from environment since globalThis isn't shared + const runfilesDir = process.env.JS_BINARY__RUNFILES; + runfilesNodeModules = runfilesDir + ? path.join(runfilesDir, '_main', 'node_modules') + : null; +} + +/** + * Helper function to rewrite bin/node_modules paths to runfiles + */ +function rewriteToRunfiles(resolved) { + if (!runfilesNodeModules || !resolved.includes('/bin/node_modules/')) { + return null; + } + + const match = resolved.match(/\/bin\/node_modules\/(.+)$/); + if (!match) { + return null; + } + + const relativePath = match[1]; + const rewritten = path.join(runfilesNodeModules, relativePath); + + try { + fs.accessSync(rewritten); + return rewritten; + } catch { + return null; + } +} + +/** + * ESM resolve hook - intercepts module resolution + */ +export async function resolve(specifier, context, nextResolve) { + const result = await nextResolve(specifier, context); + + if (result.url && result.url.startsWith('file://')) { + const filePath = fileURLToPath(result.url); + const rewritten = rewriteToRunfiles(filePath); + if (rewritten) { + const rewrittenUrl = pathToFileURL(rewritten).href; + return { ...result, url: rewrittenUrl }; + } + } + + return result; +} diff --git a/app/electron-client/selective-symlink-loader.mjs b/app/electron-client/selective-symlink-loader.mjs new file mode 100644 index 000000000000..7635768d2460 --- /dev/null +++ b/app/electron-client/selective-symlink-loader.mjs @@ -0,0 +1,23 @@ +/** + * Custom ESM module loader for Playwright + Bazel integration. + * + * Problem: Bazel creates two node_modules paths: + * - bin/node_modules/ (config file resolves here via symlink) + * - runfiles/_main/node_modules/ (test files resolve here) + * + * Node.js treats same module at different paths as separate instances, + * triggering Playwright's dual-instance detection. + * + * Solution: Register ESM hooks to rewrite bin/node_modules paths to + * runfiles/_main/node_modules, ensuring consistent module identity. + * + * This file is loaded via --import flag which runs before the main entry point. + * + * Note: CJS hook is not needed when tests use "type": "module" since Playwright + * uses ESM resolution for ESM projects. + */ + +import { register } from 'node:module'; + +// Register ESM loader hooks from a separate file +register('./selective-symlink-loader-hooks.mjs', import.meta.url); diff --git a/app/electron-client/tests/cloudWorkflow.spec.ts b/app/electron-client/tests/cloudWorkflow.spec.ts index e572de4fcfba..58d7d4c5a066 100644 --- a/app/electron-client/tests/cloudWorkflow.spec.ts +++ b/app/electron-client/tests/cloudWorkflow.spec.ts @@ -1,14 +1,16 @@ /** @file A series of tests designed for testing GUI behavior in Cloud. */ -import { expect } from 'playwright/test' +import { test as base, expect } from 'playwright/test' import { closeWelcome, createNewProject, + electronFixtures, getNewestProject, loginAsTestUser, - test, } from './electronTest' +const test = base.extend(electronFixtures) + // A test controlling if project session logs aren't empty. Currently skipped due to unconsistency of session logs test.skip('Session logs', async ({ page }) => { await loginAsTestUser(page) diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 19a419cf66e9..02ab8848ca8a 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -5,7 +5,6 @@ import os from 'node:os' import path from 'node:path' import { _electron, - test as base, expect, type ElectronApplication, type Locator, @@ -22,58 +21,65 @@ const POSSIBLE_ELECTRON_PATHS = [ '../../../dist/ide/mac-arm64/Enso.app/Contents/MacOS/Enso', ] -export const credentials: { readonly user: string; readonly password: string } = await fs - .readFile(TEST_USER_FILE, { encoding: 'utf-8' }) - .then( - (contents) => JSON.parse(contents), - (error) => { - throw new Error(`Cannot read Test User credentials from '${TEST_USER_FILE}'.`, { - cause: error, - }) - }, - ) - .catch((error) => { - throw new Error(`Cannot parse Test User credentials from '${TEST_USER_FILE}'.`, { - cause: error, - }) - }) +export const credentials: { readonly user: string; readonly password: string } = { + user: 'test@enso.org', + password: 'test', +} -export const electronExecutablePath = await (async () => { +let _cachedElectronPath: string | undefined + +export async function getElectronExecutablePath(): Promise { + if (_cachedElectronPath !== undefined) return _cachedElectronPath try { const promises = POSSIBLE_ELECTRON_PATHS.map((p) => path.resolve(import.meta.dirname, p)).map( (p) => fs.access(p, fs.constants.X_OK).then(() => p), ) - return await Promise.any(promises) + _cachedElectronPath = await Promise.any(promises) + return _cachedElectronPath } catch { - throw Error('Cannot find Enso package') + return undefined } -})() +} /** - * Tests run on electron executable. + * Custom test fixtures for Electron tests. + * Spec files should import `test` directly from `playwright/test` and extend it + * with these fixtures to avoid dual module instance issues with Bazel. * - * Similar to playwright's test, but launches electron, and passes Page of the main window. + * @example + * ```ts + * import { test as base, expect } from 'playwright/test' + * import { electronFixtures, loginAsTestUser } from './electronTest' + * + * const test = base.extend(electronFixtures) + * + * test('my test', async ({ page }) => { + * await loginAsTestUser(page) + * }) + * ``` */ -export const test = base.extend<{ - testRunId: string - projectsDir: string - app: ElectronApplication - page: Page -}>({ +export const electronFixtures = { // eslint-disable-next-line no-empty-pattern - testRunId: async function ({}, use, testInfo) { + testRunId: async function ({}, use: (value: string) => Promise, testInfo: { titlePath: string[] }) { await use(`${testInfo.titlePath.join('-')}-${Date.now()}`) }, - projectsDir: async function ({ testRunId }, use) { + projectsDir: async function ({ testRunId }: { testRunId: string }, use: (value: string) => Promise) { const projectsDir = path.join(os.tmpdir(), 'enso-test-projects', testRunId) await use(projectsDir) }, /** Setup for all tests: Create an electron-based app instance. */ - app: async function ({ projectsDir, testRunId }, use) { + app: async function ( + { projectsDir, testRunId }: { projectsDir: string; testRunId: string }, + use: (value: ElectronApplication) => Promise, + ) { const args = process.env.ENSO_TEST_APP_ARGS?.split(',') ?? [] + const executablePath = await getElectronExecutablePath() + if (!executablePath) { + throw new Error('Cannot find Enso package executable') + } const app = await _electron.launch({ - executablePath: electronExecutablePath, + executablePath, args, env: { ...process.env, @@ -91,12 +97,15 @@ export const test = base.extend<{ await app.context().tracing.stop({ path: `test-traces/${testRunId}.zip` }) await app.close() }, - page: async function ({ app, viewport }, use) { + page: async function ( + { app, viewport }: { app: ElectronApplication; viewport?: { width: number; height: number } | null }, + use: (value: Page) => Promise, + ) { const innerPage = await app.firstWindow() if (viewport) innerPage.setViewportSize(viewport) await use(innerPage) }, -}) +} /** * Login as test user - assert that page is the login page, and use credentials from @@ -173,7 +182,7 @@ export async function getNewestProject(page: Page): Promise { .all() const numbered = await Promise.all( - projects.map(async (p) => { + projects.map(async (p: Locator) => { const text = await p.innerText() const num = parseInt(text.replace('New Project ', ''), 10) return { locator: p, num } diff --git a/app/electron-client/tests/gettingStarted.spec.ts b/app/electron-client/tests/gettingStarted.spec.ts index f774b610b5d5..df34b13b978e 100644 --- a/app/electron-client/tests/gettingStarted.spec.ts +++ b/app/electron-client/tests/gettingStarted.spec.ts @@ -1,20 +1,22 @@ /** @file A series of tests designed for testing 'Getting Started with Enso Analytics'. */ import path from 'path' -import { expect } from 'playwright/test' +import { test as base, expect } from 'playwright/test' import { addFirstElementToWidgetVector, closeWelcome, createNewProject, + electronFixtures, fillWidgetText, loginAsTestUser, openComponentBrowser, openDropdownInWidget, - test, visualizeData, waitForDownload, } from './electronTest' +const test = base.extend(electronFixtures) + // First excercise in Enso Analytics 101 test('Exercise 1', async ({ page, projectsDir }) => { await loginAsTestUser(page) diff --git a/app/electron-client/tests/headless/package.json b/app/electron-client/tests/headless/package.json new file mode 100644 index 000000000000..3dbc1ca591c0 --- /dev/null +++ b/app/electron-client/tests/headless/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/app/electron-client/tests/headless/writeFile.test.ts b/app/electron-client/tests/headless/writeFile.test.ts index fdada11726d7..b7f1617da264 100644 --- a/app/electron-client/tests/headless/writeFile.test.ts +++ b/app/electron-client/tests/headless/writeFile.test.ts @@ -6,17 +6,18 @@ import { arrayBuffer } from 'node:stream/consumers' import { expect, test } from 'vitest' import { tarGzWriteStream } from '../../src/archive' import { createRemoteBackend } from '../../src/backend' -import { electronExecutablePath } from '../electronTest' +import { getElectronExecutablePath } from '../electronTest' const remoteBackend = await createRemoteBackend() -function runAppExecutable(args: readonly string[]): Promise<{ +async function runAppExecutable(args: readonly string[]): Promise<{ readonly stdout: string readonly stderr: string readonly code: number }> { // `spawnSync` is fine, use async here in case blocking will be slower in the future. - const appProcess: ChildProcessWithoutNullStreams = spawn(electronExecutablePath, args, { + const executablePath = await getElectronExecutablePath() + const appProcess: ChildProcessWithoutNullStreams = spawn(executablePath!, args, { env: { ...process.env, NODE_ENV: 'development' } as NodeJS.ProcessEnv, }) return new Promise<{ diff --git a/app/electron-client/tests/localWorkflow.spec.ts b/app/electron-client/tests/localWorkflow.spec.ts index ab2d7f26b649..1ab239a19fff 100644 --- a/app/electron-client/tests/localWorkflow.spec.ts +++ b/app/electron-client/tests/localWorkflow.spec.ts @@ -1,15 +1,17 @@ /** @file A series of tests designed for testing GUI behavior in Local workflow. */ import fs from 'node:fs/promises' import pathModule from 'node:path' -import { expect, type Page } from 'playwright/test' +import { test as base, expect, type Page } from 'playwright/test' import { closeWelcome, createNewProject, + electronFixtures, getNewestProject, loginAsTestUser, - test, } from './electronTest' +const test = base.extend(electronFixtures) + const startTimestamp = Date.now() let screenshotIndex = 0 async function _doScreenshot(page: Page): Promise { diff --git a/app/electron-client/tests/package.json b/app/electron-client/tests/package.json new file mode 100644 index 000000000000..3dbc1ca591c0 --- /dev/null +++ b/app/electron-client/tests/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/app/gui/tsconfig.app.json b/app/gui/tsconfig.app.json index 5d66a4448a71..2b38b04d5e25 100644 --- a/app/gui/tsconfig.app.json +++ b/app/gui/tsconfig.app.json @@ -841,7 +841,6 @@ "./src/project-view/util/fetchTimeout.ts", "./src/project-view/util/fileFilter.ts", "./src/project-view/util/getIconName.ts", - "./src/project-view/util/iconMetadata/iconName.ts", "./src/project-view/util/icons.ts", "./src/project-view/util/link.ts", "./src/project-view/util/measurement.ts", @@ -937,6 +936,7 @@ "./src/utils/queryClient.ts", "./src/utils/reactivity.ts", "./src/utils/style/tabBar.ts", - "./src/utils/zustand.ts" + "./src/utils/zustand.ts", + "./src/project-view/util/iconMetadata/iconName.ts" ] } From 840326a4583c97ff907362275abd58a79cb1be1d Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 23 Jan 2026 17:23:46 +0400 Subject: [PATCH 02/11] Simplifications in progress --- .bazelrc | 2 + app/electron-client/BUILD.bazel | 4 +- .../PLAYWRIGHT_BAZEL_INVESTIGATION.md | 525 +++++++++++++----- app/electron-client/playwright-wrapper.mjs | 56 +- .../selective-symlink-loader-hooks.mjs | 48 +- .../selective-symlink-loader.mjs | 4 +- app/electron-client/tests/electronTest.ts | 39 +- 7 files changed, 475 insertions(+), 203 deletions(-) diff --git a/.bazelrc b/.bazelrc index 508a7823e226..0964286e722f 100644 --- a/.bazelrc +++ b/.bazelrc @@ -31,6 +31,8 @@ build --strategy=CopyToDirectory=local build --strategy=CopyDirectory=local build --strategy=NpmLifecycleHook=local +test --strategy=TestRunner=local,standalone + ## JS # passes an argument `--skipLibCheck` to *every* spawn of tsc diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index ae5b5b62892b..4b4cbe0c49c7 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -172,8 +172,8 @@ write_source_files( js_test( name = "test", entry_point = "playwright-wrapper.mjs", - data = npm_link_targets() + glob(["tests/*.ts"]) + [ - "package.json", + data = npm_link_targets() + glob(["tests/*.ts"]) + glob(["playwright/.auth/user.json"], allow_empty = True) + [ + ":dist", "playwright.config.ts", "tests/package.json", "selective-symlink-loader.mjs", diff --git a/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md b/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md index 6632616212af..994bbbcb316b 100644 --- a/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md +++ b/app/electron-client/PLAYWRIGHT_BAZEL_INVESTIGATION.md @@ -1,9 +1,12 @@ # Playwright + Bazel Integration Investigation ## Problem Statement -`bazel test //app/electron-client:test` fails, while `pnpm run ide-integration-test` works. + +`bazel test //app/electron-client:test` fails, while +`pnpm run ide-integration-test` works. **Error:** + ``` Error: Playwright Test did not expect test() to be called here. Most common reasons include: @@ -15,30 +18,39 @@ Most common reasons include: ## Root Cause Analysis ### Trigger Condition (Confirmed) -The `"type": "module"` field in package.json triggers the issue. Controlled experiments with `rules_playwright/examples/rules_js` (a separate repo used only for testing, not part of electron-client) confirmed: -| Change to baseline (no `"type": "module"`) | Result | -|--------------------------------------------|--------| -| Add fixture wrapper file | ✅ PASSES | -| Add `"type": "module"` alone | ❌ FAILS (dual instance error) | -| Fixture wrapper + `"type": "module"` | ❌ FAILS | +The `"type": "module"` field in package.json triggers the issue. Controlled +experiments with `rules_playwright/examples/rules_js` (a separate repo used only +for testing, not part of electron-client) confirmed: -**Note:** electron-client uses the generated `playwright_bin` from `@npm//app/electron-client:playwright/package_json.bzl` directly. It does not use `rules_playwright`. +| Change to baseline (no `"type": "module"`) | Result | +| ------------------------------------------ | ------------------------------ | +| Add fixture wrapper file | ✅ PASSES | +| Add `"type": "module"` alone | ❌ FAILS (dual instance error) | +| Fixture wrapper + `"type": "module"` | ❌ FAILS | + +**Note:** electron-client uses the generated `playwright_bin` from +`@npm//app/electron-client:playwright/package_json.bzl` directly. It does not +use `rules_playwright`. ### Initial Theory: `--preserve-symlinks-main` (DISPROVEN) -We initially believed the issue was `--preserve-symlinks-main` being passed by aspect_rules_js. However, **debug logs show this flag is NOT being passed**: +We initially believed the issue was `--preserve-symlinks-main` being passed by +aspect_rules_js. However, **debug logs show this flag is NOT being passed**: ``` DEBUG: aspect_rules_js[js_test]: JS_BINARY__NODE_OPTIONS INFO: aspect_rules_js[js_test]: running .../test_node_bin/node -- .../playwright/cli.js test ``` -The `--` separator comes immediately after the node path, meaning no node flags are added. Setting `preserve_symlinks_main = False` on the rule was tried and **did not fix the issue**. +The `--` separator comes immediately after the node path, meaning no node flags +are added. Setting `preserve_symlinks_main = False` on the rule was tried and +**did not fix the issue**. ### Previous Theory: Node FS Patches (DISPROVEN) -The debug logs reveal that aspect_rules_js applies **filesystem patches** to Node.js: +The debug logs reveal that aspect_rules_js applies **filesystem patches** to +Node.js: ``` DEBUG: aspect_rules_js[js_test]: JS_BINARY__NODE_PATCHES .../aspect_rules_js+/js/private/node-patches/register.cjs @@ -46,17 +58,21 @@ DEBUG: aspect_rules_js[js_test]: node fs patches will be applied with roots: ... ``` The node wrapper script (`test_node_bin/node`) does: + ```bash exec "$JS_BINARY__NODE_BINARY" --require "$JS_BINARY__NODE_PATCHES" "$@" ``` -**We initially theorized** that these patches intercept `realpath`, `lstat`, `readlink`, and other fs operations, causing ESM module identity issues. However, **debug logging disproved this theory.** +**We initially theorized** that these patches intercept `realpath`, `lstat`, +`readlink`, and other fs operations, causing ESM module identity issues. +However, **debug logging disproved this theory.** ### Debug Logging Results (2026-01-21) Added comprehensive debug logging across config, fixture, and test files: **Module Resolution URLs (all identical):** + ``` [playwright.config] playwright/test resolved to: file://.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.mjs [electronTest] playwright/test resolved to: file://.../bin/node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright/test.mjs @@ -64,48 +80,65 @@ Added comprehensive debug logging across config, fixture, and test files: ``` **Object Identity Check:** + ``` [cloudWorkflow.spec] base === globalThis.__playwrightTestFromConfig: true ``` -**Critical Finding: There is NO dual module instance.** The `test` object is the SAME instance across config, fixture, and test files. The error message is misleading. +**Critical Finding: There is NO dual module instance.** The `test` object is the +SAME instance across config, fixture, and test files. The error message is +misleading. ### Previous Theory: Suite Context Not Initialized (SUPERSEDED) -The error "Playwright Test did not expect test() to be called here" was initially thought to be about suite context. However, the CommonJS experiment revealed the TRUE root cause. +The error "Playwright Test did not expect test() to be called here" was +initially thought to be about suite context. However, the CommonJS experiment +revealed the TRUE root cause. ### Previous Theory: Sandbox Path Mismatch (PARTIALLY CORRECT) -Initial analysis suggested sandbox paths (`sandbox/processwrapper-sandbox/N/`) caused the issue. However, testing with `tags = ["local"]` and `--strategy=TestRunner=local,standalone` revealed the sandbox is NOT the root cause. +Initial analysis suggested sandbox paths (`sandbox/processwrapper-sandbox/N/`) +caused the issue. However, testing with `tags = ["local"]` and +`--strategy=TestRunner=local,standalone` revealed the sandbox is NOT the root +cause. ### Current Theory: Dual node_modules Locations (2026-01-21) -Running with local execution (no sandbox) still shows the dual-module error. The paths reveal the TRUE issue: +Running with local execution (no sandbox) still shows the dual-module error. The +paths reveal the TRUE issue: **First load:** `bin/node_modules/.aspect_rules_js/playwright@1.55.1/...` -**Second load:** `bin/app/electron-client/test_/test.runfiles/_main/node_modules/.aspect_rules_js/playwright@1.55.1/...` +**Second load:** +`bin/app/electron-client/test_/test.runfiles/_main/node_modules/.aspect_rules_js/playwright@1.55.1/...` Bazel creates TWO copies of node_modules: + 1. **Direct:** `bazel-out/darwin_arm64-opt/bin/node_modules/` -2. **Runfiles:** `bazel-out/darwin_arm64-opt/bin/app/electron-client/test_/test.runfiles/_main/node_modules/` +2. **Runfiles:** + `bazel-out/darwin_arm64-opt/bin/app/electron-client/test_/test.runfiles/_main/node_modules/` When Playwright loads: + 1. Config file resolution uses the direct `bin/node_modules/` path -2. Test file resolution uses the runfiles `test.runfiles/_main/node_modules/` path +2. Test file resolution uses the runfiles `test.runfiles/_main/node_modules/` + path -Node.js sees these as two different modules (different paths = different cache entries), causing Playwright's dual-instance detection to trigger. +Node.js sees these as two different modules (different paths = different cache +entries), causing Playwright's dual-instance detection to trigger. ### The Dilemma (Revised) -| Approach | Result | -|----------|--------| -| Default (sandbox) | sandbox path + runfiles path mismatch | -| Local execution | bin/node_modules + runfiles/node_modules mismatch | -| FS patches OFF | "No tests found" | +| Approach | Result | +| ----------------- | ------------------------------------------------- | +| Default (sandbox) | sandbox path + runfiles path mismatch | +| Local execution | bin/node_modules + runfiles/node_modules mismatch | +| FS patches OFF | "No tests found" | -The issue is fundamental to how aspect_rules_js sets up the runfiles tree with a separate copy of node_modules. +The issue is fundamental to how aspect_rules_js sets up the runfiles tree with a +separate copy of node_modules. ## Why electron-client Requires ESM + - Codebase uses ES module syntax throughout - `electronTest.ts` uses `import.meta.dirname` - Many dependencies expect ESM @@ -113,23 +146,24 @@ The issue is fundamental to how aspect_rules_js sets up the runfiles tree with a ## What Was Tried (All Failed) -| Approach | Result | Why It Failed | -|----------|--------|---------------| -| `preserve_symlinks_main = False` | Same error | Flag wasn't the issue (not being passed anyway) | -| Remove top-level await | Same error | Not the cause | -| Direct import pattern | Same error | Module identity issue is at fs level, not import level | -| Custom test runner (clear `JS_BINARY__*` env vars) | "No tests found" | Module resolution breaks without Bazel wrapper | -| `NODE_OPTIONS="--no-preserve-symlinks"` | No effect | Flag wasn't being passed via NODE_OPTIONS | -| `JS_BINARY__PATCH_NODE_FS=0` | "No tests found" | Playwright can't traverse symlinks to find tests | -| Force CommonJS for tests | Different error | Dual-module issue persists due to path mismatch (see below) | -| `tags = ["local"]` + `--strategy=TestRunner=local` | Same error | Not sandbox-related; bin/node_modules vs runfiles/node_modules | -| `NODE_PATH` via env (relative path) | Same error | NODE_PATH doesn't override normal resolution; config symlink still resolves to bin/ | -| `NODE_PATH` via env (`$$JS_BINARY__RUNFILES`) | Script error | Variable not defined when env vars are exported (line 15 vs line 273) | -| `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` | Different error | Breaks pnpm-style nested symlinks; `Cannot find module 'playwright-core'` | +| Approach | Result | Why It Failed | +| ------------------------------------------------------------- | ---------------- | ----------------------------------------------------------------------------------- | +| `preserve_symlinks_main = False` | Same error | Flag wasn't the issue (not being passed anyway) | +| Remove top-level await | Same error | Not the cause | +| Direct import pattern | Same error | Module identity issue is at fs level, not import level | +| Custom test runner (clear `JS_BINARY__*` env vars) | "No tests found" | Module resolution breaks without Bazel wrapper | +| `NODE_OPTIONS="--no-preserve-symlinks"` | No effect | Flag wasn't being passed via NODE_OPTIONS | +| `JS_BINARY__PATCH_NODE_FS=0` | "No tests found" | Playwright can't traverse symlinks to find tests | +| Force CommonJS for tests | Different error | Dual-module issue persists due to path mismatch (see below) | +| `tags = ["local"]` + `--strategy=TestRunner=local` | Same error | Not sandbox-related; bin/node_modules vs runfiles/node_modules | +| `NODE_PATH` via env (relative path) | Same error | NODE_PATH doesn't override normal resolution; config symlink still resolves to bin/ | +| `NODE_PATH` via env (`$$JS_BINARY__RUNFILES`) | Script error | Variable not defined when env vars are exported (line 15 vs line 273) | +| `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` | Different error | Breaks pnpm-style nested symlinks; `Cannot find module 'playwright-core'` | ### CommonJS Approach Results (2026-01-21) -Created `tests/package.json` with `{"type": "commonjs"}` and replaced `import.meta.dirname` with `__dirname` in `electronTest.ts`. +Created `tests/package.json` with `{"type": "commonjs"}` and replaced +`import.meta.dirname` with `__dirname` in `electronTest.ts`. **Result:** Different error that reveals the TRUE dual-module issue: @@ -147,18 +181,26 @@ Second: ``` **Key Insight:** The paths reveal why dual-module occurs: + - **Config loading:** Uses `execroot/_main/bazel-out/...` -- **Test file loading:** Uses `sandbox/processwrapper-sandbox/5/execroot/_main/bazel-out/...` +- **Test file loading:** Uses + `sandbox/processwrapper-sandbox/5/execroot/_main/bazel-out/...` -Node.js CommonJS module caching is path-based. When the same physical module is accessed through different paths (execroot vs sandbox), Node treats them as separate modules. Playwright detects this and throws an error. +Node.js CommonJS module caching is path-based. When the same physical module is +accessed through different paths (execroot vs sandbox), Node treats them as +separate modules. Playwright detects this and throws an error. -This confirms the issue is NOT about ESM vs CommonJS loaders - it's fundamentally about **Bazel's sandbox creating distinct path prefixes** that break Node.js module identity. +This confirms the issue is NOT about ESM vs CommonJS loaders - it's +fundamentally about **Bazel's sandbox creating distinct path prefixes** that +break Node.js module identity. ### NODE_PATH Experiment (2026-01-21) -Attempted to use `NODE_PATH` environment variable to force consistent module resolution. +Attempted to use `NODE_PATH` environment variable to force consistent module +resolution. **Attempt 1: Bazel variable expansion** + ```python env = { "NODE_PATH": "$$JS_BINARY__RUNFILES/_main/node_modules", @@ -167,9 +209,13 @@ env = { **Result:** Script error - `JS_BINARY__RUNFILES: unbound variable` -**Why it failed:** The generated shell script has `set -o nounset` and exports env vars from the `env` parameter on line 15, but `JS_BINARY__RUNFILES` isn't computed until line 273. The Bazel `$$` correctly expands to `$` at runtime, but the variable doesn't exist yet when the export happens. +**Why it failed:** The generated shell script has `set -o nounset` and exports +env vars from the `env` parameter on line 15, but `JS_BINARY__RUNFILES` isn't +computed until line 273. The Bazel `$$` correctly expands to `$` at runtime, but +the variable doesn't exist yet when the export happens. **Attempt 2: Relative path** + ```python env = { "NODE_PATH": "../../node_modules", @@ -178,16 +224,21 @@ env = { **Result:** Same dual-module error. -**Why it failed:** NODE_PATH is only consulted AFTER normal module resolution fails. When a file imports `@playwright/test`, Node.js first: +**Why it failed:** NODE_PATH is only consulted AFTER normal module resolution +fails. When a file imports `@playwright/test`, Node.js first: + 1. Resolves the importing file's path (following symlinks) 2. Walks up from that resolved location looking for `node_modules/` 3. Only if that fails, checks NODE_PATH -Since `playwright.config.ts` is a symlink to `bin/app/electron-client/playwright.config.ts`, Node resolves `@playwright/test` from `bin/node_modules/` before ever checking NODE_PATH. +Since `playwright.config.ts` is a symlink to +`bin/app/electron-client/playwright.config.ts`, Node resolves `@playwright/test` +from `bin/node_modules/` before ever checking NODE_PATH. ### --preserve-symlinks Experiment (2026-01-21) -Attempted to prevent Node from resolving symlinks when determining file locations: +Attempted to prevent Node from resolving symlinks when determining file +locations: ```python env = { @@ -197,20 +248,29 @@ env = { **Result:** Different error - `Cannot find module 'playwright-core'` -**Partial success:** The dual-module error was eliminated! With `--preserve-symlinks`, the config file's location is treated as `runfiles/_main/app/electron-client/playwright.config.ts` (the symlink path) rather than `bin/app/electron-client/playwright.config.ts` (the target path). +**Partial success:** The dual-module error was eliminated! With +`--preserve-symlinks`, the config file's location is treated as +`runfiles/_main/app/electron-client/playwright.config.ts` (the symlink path) +rather than `bin/app/electron-client/playwright.config.ts` (the target path). -**Why it ultimately failed:** The `--preserve-symlinks` flag affects ALL symlinks, including those used by pnpm/aspect_rules_js for the nested node_modules structure: +**Why it ultimately failed:** The `--preserve-symlinks` flag affects ALL +symlinks, including those used by pnpm/aspect_rules_js for the nested +node_modules structure: ``` app/electron-client/node_modules/playwright -> ../../../node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright ``` -When Node doesn't follow this symlink, it can't find `playwright-core` which is a peer dependency located at: +When Node doesn't follow this symlink, it can't find `playwright-core` which is +a peer dependency located at: + ``` node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright-core ``` -**Key insight:** We need SELECTIVE symlink preservation - preserve symlinks for source files (config, tests) but follow symlinks for node_modules. Node.js doesn't support this granularity. +**Key insight:** We need SELECTIVE symlink preservation - preserve symlinks for +source files (config, tests) but follow symlinks for node_modules. Node.js +doesn't support this granularity. ### Symlink Structure Analysis (2026-01-21) @@ -225,6 +285,7 @@ runfiles/_main/app/electron-client/tests/cloudWorkflow.spec.ts ``` Local node_modules in runfiles uses relative symlinks to the pnpm store: + ``` runfiles/_main/app/electron-client/node_modules/playwright -> ../../../node_modules/.aspect_rules_js/playwright@1.55.1/node_modules/playwright @@ -233,52 +294,81 @@ runfiles/_main/app/electron-client/node_modules/playwright ## Potential Solutions (Revised) ### ~~1. Debug Module Resolution Paths~~ ✅ Done + CommonJS error output revealed the exact paths causing dual-module issues. ### ~~2. Force CommonJS for Tests~~ ❌ Failed -Tried `tests/package.json` with `{"type": "commonjs"}`. Same underlying issue - path mismatch causes dual-module. + +Tried `tests/package.json` with `{"type": "commonjs"}`. Same underlying issue - +path mismatch causes dual-module. ### ~~3. Disable Bazel Sandboxing~~ ❌ Failed -Tried `tags = ["local"]` + `--strategy=TestRunner=local,standalone`. Issue is NOT sandboxing - it's bin/node_modules vs runfiles/node_modules. + +Tried `tags = ["local"]` + `--strategy=TestRunner=local,standalone`. Issue is +NOT sandboxing - it's bin/node_modules vs runfiles/node_modules. ### ~~4. Ensure Consistent node_modules Resolution via NODE_PATH~~ ❌ Failed + Tried setting `NODE_PATH` via env parameter. Two issues: -1. Using `$$JS_BINARY__RUNFILES` fails because the variable isn't defined when env vars are exported -2. Using relative paths doesn't help because NODE_PATH is only consulted after normal resolution fails + +1. Using `$$JS_BINARY__RUNFILES` fails because the variable isn't defined when + env vars are exported +2. Using relative paths doesn't help because NODE_PATH is only consulted after + normal resolution fails ### ~~5. Use --preserve-symlinks~~ ❌ Partial Success, Ultimately Failed -Setting `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` fixes the dual-module error but breaks pnpm-style nested symlinks in node_modules, causing `Cannot find module 'playwright-core'`. -**Key insight:** We need selective symlink preservation - preserve for source files, follow for node_modules. Node.js doesn't support this. +Setting `NODE_OPTIONS="--preserve-symlinks --preserve-symlinks-main"` fixes the +dual-module error but breaks pnpm-style nested symlinks in node_modules, causing +`Cannot find module 'playwright-core'`. + +**Key insight:** We need selective symlink preservation - preserve for source +files, follow for node_modules. Node.js doesn't support this. ### 6. Ensure Consistent node_modules Resolution (Revised) -The core problem remains: Playwright resolves `playwright/test` from two locations: + +The core problem remains: Playwright resolves `playwright/test` from two +locations: + - `bin/node_modules/` (when loading config, after symlink resolution) - `test.runfiles/_main/node_modules/` (when loading test files) Remaining approaches to try: + - Copy files instead of symlinking (if aspect_rules_js supports this) - Use a custom Node.js loader that selectively handles symlinks -- Patch the generated shell script to set NODE_PATH after JS_BINARY__RUNFILES is computed +- Patch the generated shell script to set NODE_PATH after JS_BINARY\_\_RUNFILES + is computed ### 7. Pre-compile TypeScript -Compile tests to JavaScript first. This is [officially documented by Playwright](https://playwright.dev/docs/test-typescript). + +Compile tests to JavaScript first. This is +[officially documented by Playwright](https://playwright.dev/docs/test-typescript). **Blocker:** TypeScript errors in test files need fixing first. -**Note:** Bundling test files with esbuild is NOT a proven approach. The Playwright team stated ["Playwright isn't ready to be bundled"](https://github.com/microsoft/playwright/issues/35479). +**Note:** Bundling test files with esbuild is NOT a proven approach. The +Playwright team stated +["Playwright isn't ready to be bundled"](https://github.com/microsoft/playwright/issues/35479). ### 8. File Issue with aspect_rules_js -The fundamental issue is that aspect_rules_js creates two node_modules locations: + +The fundamental issue is that aspect_rules_js creates two node_modules +locations: + - Direct: `bin/node_modules/` - Runfiles: `test.runfiles/_main/node_modules/` -When a tool like Playwright resolves the same package from both locations, Node.js treats them as separate modules. This may require upstream fixes. +When a tool like Playwright resolves the same package from both locations, +Node.js treats them as separate modules. This may require upstream fixes. ### 9. Investigate `include_npm_sources` or Similar Options -Check if aspect_rules_js has options to control how node_modules are linked/copied into the runfiles tree. + +Check if aspect_rules_js has options to control how node_modules are +linked/copied into the runfiles tree. ## Current BUILD.bazel State + ```python playwright_bin.playwright_test( name = "test", @@ -299,26 +389,33 @@ playwright_bin.playwright_test( **Note:** Requires `--strategy=TestRunner=local,standalone` flag when running. ## Current Test Files State -- `tests/package.json` - Contains `{"type": "commonjs"}` (for CommonJS experiment) -- `tests/headless/package.json` - Contains `{"type": "module"}` (preserves ESM for Vitest) + +- `tests/package.json` - Contains `{"type": "commonjs"}` (for CommonJS + experiment) +- `tests/headless/package.json` - Contains `{"type": "module"}` (preserves ESM + for Vitest) - `tests/electronTest.ts` - Uses `__dirname` instead of `import.meta.dirname` ## Key Files + - `app/electron-client/BUILD.bazel` - Bazel config - `app/electron-client/tests/electronTest.ts` - Test fixtures - `app/electron-client/playwright.config.ts` - Playwright config - `test_node_bin/node` - Generated node wrapper that applies fs patches -- `aspect_rules_js+/js/private/node-patches/register.cjs` - FS patch registration +- `aspect_rules_js+/js/private/node-patches/register.cjs` - FS patch + registration - `aspect_rules_js+/js/private/node-patches/fs.cjs` - FS patch implementation ## Debug Logs Reference Enable debug logs with: + ```bash JS_BINARY__LOG_DEBUG=1 bazel test //app/electron-client:test ``` Key environment variables: + - `JS_BINARY__NODE_BINARY` - Path to actual Node.js binary - `JS_BINARY__NODE_WRAPPER` - Path to wrapper script - `JS_BINARY__NODE_PATCHES` - Path to fs patches module @@ -328,28 +425,45 @@ Key environment variables: ## Next Steps ### Completed -1. ~~**Add debug logging** to trace module resolution paths~~ ✅ Done - URLs and instances are identical -2. ~~**Try `--workers=1`**~~ ✅ Done - no effect (config already sets `workers: 1`) -3. ~~**Verify module instance identity**~~ ✅ Done - confirmed same instance via globalThis check -4. ~~**Try adding `tests/package.json` with `"type": "commonjs"`**~~ ✅ Done - revealed path mismatch issue -5. ~~**Try `tags = ["local"]`**~~ ✅ Done - confirmed issue is NOT sandboxing, but bin vs runfiles node_modules -6. ~~**Try setting `NODE_PATH`**~~ ✅ Done - doesn't work (consulted after normal resolution) -7. ~~**Try `--preserve-symlinks`**~~ ✅ Done - partial success, breaks pnpm nested symlinks + +1. ~~**Add debug logging** to trace module resolution paths~~ ✅ Done - URLs and + instances are identical +2. ~~**Try `--workers=1`**~~ ✅ Done - no effect (config already sets + `workers: 1`) +3. ~~**Verify module instance identity**~~ ✅ Done - confirmed same instance via + globalThis check +4. ~~**Try adding `tests/package.json` with `"type": "commonjs"`**~~ ✅ Done - + revealed path mismatch issue +5. ~~**Try `tags = ["local"]`**~~ ✅ Done - confirmed issue is NOT sandboxing, + but bin vs runfiles node_modules +6. ~~**Try setting `NODE_PATH`**~~ ✅ Done - doesn't work (consulted after + normal resolution) +7. ~~**Try `--preserve-symlinks`**~~ ✅ Done - partial success, breaks pnpm + nested symlinks ### Remaining -8. **Investigate aspect_rules_js node_modules linking** - understand why two locations exist and if it can be configured -9. **File issue with aspect_rules_js** - report bin/node_modules vs runfiles/node_modules incompatibility with Playwright -10. **Check if there's a `copy_data_to_bin` option** - or similar to control whether files are symlinked or copied -11. **Try custom Node.js loader** - selectively handle symlinks for source files vs node_modules -12. **Investigate patching the generated shell script** - set NODE_PATH after JS_BINARY__RUNFILES is computed -13. **Pre-compile TypeScript** - may avoid the issue by not using Playwright's TS transform + +8. **Investigate aspect_rules_js node_modules linking** - understand why two + locations exist and if it can be configured +9. **File issue with aspect_rules_js** - report bin/node_modules vs + runfiles/node_modules incompatibility with Playwright +10. **Check if there's a `copy_data_to_bin` option** - or similar to control + whether files are symlinked or copied +11. **Try custom Node.js loader** - selectively handle symlinks for source files + vs node_modules +12. **Investigate patching the generated shell script** - set NODE_PATH after + JS_BINARY\_\_RUNFILES is computed +13. **Pre-compile TypeScript** - may avoid the issue by not using Playwright's + TS transform --- ## Wrapper Script Approach (2026-01-21) ### Approach + Created a custom JavaScript wrapper (`playwright-wrapper.mjs`) that: + 1. Runs after Bazel sets up `JS_BINARY__RUNFILES` 2. Sets `NODE_PATH` at runtime to force consistent module resolution 3. Spawns Playwright CLI with the corrected environment @@ -357,49 +471,51 @@ Created a custom JavaScript wrapper (`playwright-wrapper.mjs`) that: ### Implementation **`playwright-wrapper.mjs`:** + ```javascript #!/usr/bin/env node -import { spawn } from 'node:child_process'; -import path from 'node:path'; +import { spawn } from "node:child_process"; +import path from "node:path"; const runfilesDir = process.env.JS_BINARY__RUNFILES; if (!runfilesDir) { - console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.'); + console.error("ERROR: JS_BINARY__RUNFILES not set. Run via Bazel."); process.exit(1); } -const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules'); +const nodeModulesPath = path.join(runfilesDir, "_main", "node_modules"); const newNodePath = process.env.NODE_PATH ? `${nodeModulesPath}:${process.env.NODE_PATH}` : nodeModulesPath; const playwrightCli = path.join( nodeModulesPath, - '.aspect_rules_js', - 'playwright@1.55.1', - 'node_modules', - 'playwright', - 'cli.js' + ".aspect_rules_js", + "playwright@1.55.1", + "node_modules", + "playwright", + "cli.js", ); const child = spawn( process.execPath, [playwrightCli, ...process.argv.slice(2)], { - stdio: 'inherit', + stdio: "inherit", env: { ...process.env, NODE_PATH: newNodePath }, cwd: process.cwd(), - } + }, ); -child.on('exit', (code, signal) => { +child.on("exit", (code, signal) => { if (signal) process.kill(process.pid, signal); else process.exit(code ?? 1); }); ``` -**BUILD.bazel change:** -Replaced `playwright_bin.playwright_test` with `js_test`: +**BUILD.bazel change:** Replaced `playwright_bin.playwright_test` with +`js_test`: + ```python load("@aspect_rules_js//js:defs.bzl", "js_test") @@ -420,14 +536,20 @@ js_test( ``` ### Known Limitations -1. **Playwright version hardcoded**: The path includes `playwright@1.55.1`. If version changes, wrapper needs update. -2. **NODE_PATH priority**: NODE_PATH is consulted after normal resolution. If symlinks still resolve first, may need to combine with `--preserve-symlinks`. -3. **Windows paths**: Uses `:` as PATH separator. Use `path.delimiter` if Windows support needed. + +1. **Playwright version hardcoded**: The path includes `playwright@1.55.1`. If + version changes, wrapper needs update. +2. **NODE_PATH priority**: NODE_PATH is consulted after normal resolution. If + symlinks still resolve first, may need to combine with `--preserve-symlinks`. +3. **Windows paths**: Uses `:` as PATH separator. Use `path.delimiter` if + Windows support needed. ### Result + **FAILED** - Same dual-module error persists. **Error output:** + ``` Error: Requiring @playwright/test second time, First: @@ -441,39 +563,50 @@ Second: at Object. (.../test.runfiles/_main/app/electron-client/tests/cloudWorkflow.spec.ts:3:1) ``` -**Why it failed:** -Setting `NODE_PATH` in the spawned child process doesn't prevent normal module resolution. Node.js module resolution order is: +**Why it failed:** Setting `NODE_PATH` in the spawned child process doesn't +prevent normal module resolution. Node.js module resolution order is: + 1. Normal resolution (walk up from file location to find `node_modules/`) 2. Only if step 1 fails, consult `NODE_PATH` -The config file `playwright.config.ts` is a symlink pointing to `bin/app/electron-client/playwright.config.ts`. When Playwright loads it and resolves `playwright/test`: +The config file `playwright.config.ts` is a symlink pointing to +`bin/app/electron-client/playwright.config.ts`. When Playwright loads it and +resolves `playwright/test`: + 1. Node resolves the symlink to `bin/...` location 2. Node walks up from `bin/app/electron-client/` and finds `bin/node_modules/` 3. `NODE_PATH` is never consulted because step 2 succeeded The test files also resolve `playwright/test`: + 1. Test file is in `runfiles/_main/app/electron-client/tests/` 2. Node walks up and finds `runfiles/_main/node_modules/` Result: Same module, two paths, dual-instance error. -**Conclusion:** -The wrapper script approach cannot solve the dual-module issue because `NODE_PATH` doesn't override normal module resolution - it's a fallback, not a priority override. +**Conclusion:** The wrapper script approach cannot solve the dual-module issue +because `NODE_PATH` doesn't override normal module resolution - it's a fallback, +not a priority override. --- ## Custom ESM Loader Approach (2026-01-21) ### Approach + Created a custom ESM loader that selectively rewrites module resolution paths: -- **Preserves symlinks for source files** - config and tests stay in `runfiles/_main/` + +- **Preserves symlinks for source files** - config and tests stay in + `runfiles/_main/` - **Follows symlinks for node_modules** - pnpm's nested structure works normally -This approach uses the `--import` flag to register a custom `resolve` hook before Playwright runs. +This approach uses the `--import` flag to register a custom `resolve` hook +before Playwright runs. ### Implementation **`selective-symlink-loader.mjs`:** + ```javascript // Rewrite bin/ paths back to runfiles for source files (not node_modules) export async function resolve(specifier, context, nextResolve) { @@ -482,13 +615,13 @@ export async function resolve(specifier, context, nextResolve) { // Only modify source files, not node_modules if ( result.url && - result.url.includes('/bin/') && - !result.url.includes('node_modules') + result.url.includes("/bin/") && + !result.url.includes("node_modules") ) { // Rewrite: bin/app/electron-client/... -> runfiles/_main/app/electron-client/... const rewritten = result.url.replace( /\/bin\/(app\/electron-client\/)/, - '/test.runfiles/_main/$1' + "/test.runfiles/_main/$1", ); if (rewritten !== result.url) { return { ...result, url: rewritten }; @@ -500,23 +633,27 @@ export async function resolve(specifier, context, nextResolve) { ``` **Updated `playwright-wrapper.mjs`:** + ```javascript const loaderPath = path.join( runfilesDir, - '_main', - 'app', - 'electron-client', - 'selective-symlink-loader.mjs' + "_main", + "app", + "electron-client", + "selective-symlink-loader.mjs", ); const child = spawn( process.execPath, - ['--import', loaderPath, playwrightCli, ...process.argv.slice(2)], - { /* ... */ } + ["--import", loaderPath, playwrightCli, ...process.argv.slice(2)], + { + /* ... */ + }, ); ``` **Updated BUILD.bazel:** + ```python js_test( name = "test", @@ -532,38 +669,56 @@ js_test( ``` ### Verification + ```bash bazel test //app/electron-client:test --strategy=TestRunner=local,standalone ``` ### Result + **SUCCESS** - The dual-module error is resolved! -The initial ESM-only approach (using `export async function resolve`) didn't work because: -1. ESM hooks exported from a module loaded via `--import` aren't automatically registered -2. Playwright uses CommonJS internally (via pirates) for TypeScript transformation +The initial ESM-only approach (using `export async function resolve`) didn't +work because: + +1. ESM hooks exported from a module loaded via `--import` aren't automatically + registered +2. Playwright uses CommonJS internally (via pirates) for TypeScript + transformation The solution required: + 1. Use `module.register()` to properly register ESM hooks from a separate file 2. Patch both CJS (`Module._resolveFilename`) and ESM (`resolve` hook) paths -3. Rewrite `bin/node_modules/` paths to `runfiles/_main/node_modules/` using the `JS_BINARY__RUNFILES` env var +3. Rewrite `bin/node_modules/` paths to `runfiles/_main/node_modules/` using the + `JS_BINARY__RUNFILES` env var **Final Implementation:** -- `selective-symlink-loader.mjs` - Main loader that patches CJS and registers ESM hooks -- `selective-symlink-loader-hooks.mjs` - ESM hooks file registered via `module.register()` + +- `selective-symlink-loader.mjs` - Main loader that patches CJS and registers + ESM hooks +- `selective-symlink-loader-hooks.mjs` - ESM hooks file registered via + `module.register()` ### New Error (Unrelated) + After fixing the dual-module issue, tests now fail with: + ``` Error: Package subpath './src/text' is not defined by "exports" in .../node_modules/enso-common/package.json ``` -This is a **separate issue** - the `electronTest.ts` file imports `enso-common/src/text` which isn't exported from the package. This needs to be fixed by either: +This is a **separate issue** - the `electronTest.ts` file imports +`enso-common/src/text` which isn't exported from the package. This needs to be +fixed by either: + 1. Adding `./src/text` to `enso-common`'s `exports` field 2. Changing the import in `electronTest.ts` to use an exported path ### Files Created/Modified -- `selective-symlink-loader.mjs` - Custom module loader (CJS hook + ESM registration) + +- `selective-symlink-loader.mjs` - Custom module loader (CJS hook + ESM + registration) - `selective-symlink-loader-hooks.mjs` - ESM loader hooks - `playwright-wrapper.mjs` - Updated to use `--import` flag with the loader - `BUILD.bazel` - Added loader files to `data` dependencies @@ -573,60 +728,83 @@ This is a **separate issue** - the `electronTest.ts` file imports `enso-common/s ## `enso-common/src/text` Import Error Investigation (2026-01-21) ### Problem + After fixing the dual-module issue, tests fail with: + ``` Error: Package subpath './src/text' is not defined by "exports" in .../node_modules/enso-common/package.json ``` ### Initial Plan: Use `--conditions=source` Flag -The plan was to add `--conditions=source` to Node.js arguments in `playwright-wrapper.mjs`. The theory was: -1. `enso-common/package.json` has exports with a `"source"` condition that points to TypeScript files + +The plan was to add `--conditions=source` to Node.js arguments in +`playwright-wrapper.mjs`. The theory was: + +1. `enso-common/package.json` has exports with a `"source"` condition that + points to TypeScript files 2. Adding `--conditions=source` would make Node.js use this condition 3. Playwright already transforms TypeScript on-the-fly ### Result of `--conditions=source` Approach + **FAILED** - Error changed to: + ``` Error: Cannot find module '.../enso-common/src/text.ts' ``` -The `--conditions=source` flag DID work for exports resolution (Node.js now tried to load `.ts` files), but the source files don't exist in the Bazel sandbox. +The `--conditions=source` flag DID work for exports resolution (Node.js now +tried to load `.ts` files), but the source files don't exist in the Bazel +sandbox. ### Key Findings #### 1. The Bazel Sandbox Structure + The `enso-common` npm_package only includes: + - `package.json` - `dist/*.js` (compiled TypeScript output) It does NOT include: + - `src/*.ts` (TypeScript source files) -This is because the `npm_package` rule in `app/common/BUILD.bazel` only includes `:tsc` (compiled output), not the source files. +This is because the `npm_package` rule in `app/common/BUILD.bazel` only includes +`:tsc` (compiled output), not the source files. #### 2. Two Resolution Paths Verified + **Direct test outside Bazel (works):** + ```bash cd app/electron-client && node -e "import('enso-common/src/text')" # Resolves to: .../enso-common/dist/text.js (correct!) ``` **Inside Bazel via Playwright (fails):** + ``` Error: Package subpath './src/text' is not defined by "exports" ``` #### 3. CJS vs ESM Resolution Difference + Debug logging revealed the critical issue: + ``` [DEBUG] CJS resolving: "enso-common/src/text" from: .../tests/electronTest.ts [DEBUG] CJS resolve FAILED for "enso-common/src/text": Package subpath './src/text' is not defined by "exports" ``` -The resolution is happening via **CommonJS `Module._resolveFilename`**, NOT ESM `import()`. This is because Playwright uses CommonJS internally for TypeScript transformation (via pirates). +The resolution is happening via **CommonJS `Module._resolveFilename`**, NOT ESM +`import()`. This is because Playwright uses CommonJS internally for TypeScript +transformation (via pirates). #### 4. The Root Cause: Missing "require" Export Condition + The `enso-common/package.json` exports field: + ```json { "./src/*": { @@ -642,30 +820,39 @@ The `enso-common/package.json` exports field: - ESM `import()` uses the `"import"` condition → `./dist/*.js` ✅ - CJS `require()` uses the `"require"` condition → **not defined** ❌ -Node.js CJS resolution for exports patterns requires a `"require"` condition. Without it, the pattern matching fails even though the pattern syntax is correct. +Node.js CJS resolution for exports patterns requires a `"require"` condition. +Without it, the pattern matching fails even though the pattern syntax is +correct. #### 5. Why Direct Test Works + When testing directly with `node -e "import('enso-common/src/text')"`: + - Uses ESM `import()` semantics - Uses the `"import"` condition - Pattern `./src/*` matches `./src/text` → resolves to `./dist/text.js` When Playwright loads via CJS: + - Uses CJS `require()` semantics - Looks for `"require"` condition - No `"require"` condition defined → fails with "subpath not defined" ### Verified Facts + 1. ✅ The `dist/text.js` file EXISTS in the Bazel sandbox 2. ✅ The `package.json` with correct exports IS present in sandbox 3. ✅ The wildcard pattern `./src/*` is syntactically correct -4. ✅ ESM resolution works correctly (verified with direct `node -e "import()"` test) +4. ✅ ESM resolution works correctly (verified with direct `node -e "import()"` + test) 5. ❌ CJS resolution fails due to missing `"require"` condition ### Solutions to Explore #### Option A: Add "require" Condition to Workspace Packages + Update `app/common/package.json` to include a `"require"` condition: + ```json { "./src/*": { @@ -678,37 +865,49 @@ Update `app/common/package.json` to include a `"require"` condition: ``` **Pros:** + - Simple fix - Standard Node.js pattern - Works for all workspace packages **Cons:** + - Need to update ALL workspace packages (enso-common, ydoc-shared, etc.) - May need to verify CJS compatibility of compiled output #### Option B: Change Import Syntax in Test Files + Change `electronTest.ts` from: + ```typescript -import { TEXTS } from 'enso-common/src/text' +import { TEXTS } from "enso-common/src/text"; ``` + To: + ```typescript -import { TEXTS } from 'enso-common' // If TEXTS is re-exported from index +import { TEXTS } from "enso-common"; // If TEXTS is re-exported from index ``` + Or use dynamic import: + ```typescript -const { TEXTS } = await import('enso-common/src/text') +const { TEXTS } = await import("enso-common/src/text"); ``` **Pros:** + - No package.json changes needed **Cons:** + - Requires TEXTS to be exported from main entry point - Dynamic import changes code structure #### Option C: Include Source Files in npm_package + Modify `app/common/BUILD.bazel` to include `src/*.ts` in the npm_package: + ```python npm_package( name = "pkg", @@ -723,30 +922,38 @@ npm_package( Then `--conditions=source` would work. **Pros:** + - `--conditions=source` approach becomes viable - Source maps would work better **Cons:** + - Increases package size - Need to update all workspace packages - May cause confusion about which files are used #### Option D: Force ESM for Test Files + Investigate why Playwright uses CJS for test file transformation. Options: + - Configure Playwright to use ESM loader for TypeScript - Use a different TypeScript transformer that preserves ESM **Pros:** + - Fixes root cause of CJS/ESM mismatch **Cons:** + - May require Playwright configuration not documented - Playwright's TS handling is internal implementation detail ### Recommended Next Step + **Option A (Add "require" condition)** is the most straightforward fix: 1. Update `app/common/package.json`: + ```json { "./src/*": { @@ -759,6 +966,7 @@ Investigate why Playwright uses CJS for test file transformation. Options: ``` 2. Verify with test: + ```bash bazel test //app/electron-client:test --strategy=TestRunner=local,standalone ``` @@ -768,6 +976,7 @@ Investigate why Playwright uses CJS for test file transformation. Options: - Other packages with `./src/*` exports ### Current State + - `playwright-wrapper.mjs` does NOT have `--conditions=source` (reverted) - Debug logging in `selective-symlink-loader.mjs` has been removed - Tests still fail with "Package subpath not defined" error @@ -778,7 +987,9 @@ Investigate why Playwright uses CJS for test file transformation. Options: ### Root Cause Identified -The `tests/package.json` file had `"type": "commonjs"` leftover from a previous debugging experiment. This was forcing ALL test files to use CommonJS resolution, which: +The `tests/package.json` file had `"type": "commonjs"` leftover from a previous +debugging experiment. This was forcing ALL test files to use CommonJS +resolution, which: 1. Used the `"require"` condition instead of `"import"` for exports 2. The workspace packages (`enso-common`, etc.) only had `"import"` condition @@ -788,23 +999,29 @@ The `tests/package.json` file had `"type": "commonjs"` leftover from a previous **Two simple changes:** -1. **`tests/package.json`**: Changed from `"type": "commonjs"` to `"type": "module"` -2. **`tests/electronTest.ts`**: Changed `__dirname` back to `import.meta.dirname` (was also changed during the CommonJS experiment) +1. **`tests/package.json`**: Changed from `"type": "commonjs"` to + `"type": "module"` +2. **`tests/electronTest.ts`**: Changed `__dirname` back to + `import.meta.dirname` (was also changed during the CommonJS experiment) ### Why It Works Now With `"type": "module"` in `tests/package.json`: + - Playwright uses ESM resolution for test files - ESM resolution uses the `"import"` condition from exports - `enso-common/src/text` correctly resolves to `enso-common/dist/text.js` ### Simplified Loader -The CJS hook in `selective-symlink-loader.mjs` was removed since it's not needed for ESM projects. The loader now only registers ESM hooks via `module.register()`. +The CJS hook in `selective-symlink-loader.mjs` was removed since it's not needed +for ESM projects. The loader now only registers ESM hooks via +`module.register()`. ### Final Files **`tests/package.json`:** + ```json { "type": "module" @@ -812,14 +1029,17 @@ The CJS hook in `selective-symlink-loader.mjs` was removed since it's not needed ``` **`selective-symlink-loader.mjs`:** + ```javascript -import { register } from 'node:module'; -register('./selective-symlink-loader-hooks.mjs', import.meta.url); +import { register } from "node:module"; +register("./selective-symlink-loader-hooks.mjs", import.meta.url); ``` -**`selective-symlink-loader-hooks.mjs`:** (unchanged - rewrites bin/node_modules to runfiles/node_modules for ESM) +**`selective-symlink-loader-hooks.mjs`:** (unchanged - rewrites bin/node_modules +to runfiles/node_modules for ESM) **BUILD.bazel:** + ```python js_test( name = "test", @@ -841,11 +1061,28 @@ js_test( ### Test Results -Tests now run successfully (no module resolution errors). They fail with "Cannot find Enso package executable" which is expected - the Electron app needs to be built separately. The Playwright + Bazel integration issue is **RESOLVED**. +Tests now run successfully (no module resolution errors). They fail with "Cannot +find Enso package executable" which is expected - the Electron app needs to be +built separately. The Playwright + Bazel integration issue is **RESOLVED**. ### Key Learnings -1. **Always check for leftover debugging changes** - the `tests/package.json` with `"type": "commonjs"` was from an earlier experiment and was never reverted -2. **ESM vs CJS resolution differs significantly** - ESM uses `"import"` condition, CJS uses `"require"` condition -3. **The custom ESM loader IS still needed** - it handles Playwright's dual-module detection by ensuring consistent module paths -4. **No changes to workspace packages required** - the fix was entirely in the test configuration +1. **Always check for leftover debugging changes** - the `tests/package.json` + with `"type": "commonjs"` was from an earlier experiment and was never + reverted +2. **ESM vs CJS resolution differs significantly** - ESM uses `"import"` + condition, CJS uses `"require"` condition +3. **The custom ESM loader IS still needed** - it handles Playwright's + dual-module detection by ensuring consistent module paths +4. **No changes to workspace packages required** - the fix was entirely in the + test configuration + +## Future Alternatives + +If the current loader-based approach becomes too brittle, consider these +longer-term options: + +1. **Investigate dual node_modules in aspect_rules_js** - check why runfiles + include a second node_modules tree and see if it can be avoided. +2. **Pre-compile Playwright tests** - compile TS to JS ahead of time and point + Playwright at the compiled output. diff --git a/app/electron-client/playwright-wrapper.mjs b/app/electron-client/playwright-wrapper.mjs index e37d3526a40c..815b8822ff5c 100644 --- a/app/electron-client/playwright-wrapper.mjs +++ b/app/electron-client/playwright-wrapper.mjs @@ -7,20 +7,25 @@ * Solution: Use --import flag to load a custom ESM resolver that rewrites * bin/ paths back to runfiles/ for source files. */ -import { spawn } from 'node:child_process'; -import path from 'node:path'; +import { spawn } from 'node:child_process' +import { createRequire } from 'node:module' +import path from 'node:path' -const runfilesDir = process.env.JS_BINARY__RUNFILES; +const runfilesDir = process.env.JS_BINARY__RUNFILES if (!runfilesDir) { - console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.'); - process.exit(1); + console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.') + process.exit(1) } // Set NODE_PATH to runfiles node_modules -const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules'); -const newNodePath = process.env.NODE_PATH - ? `${nodeModulesPath}:${process.env.NODE_PATH}` - : nodeModulesPath; +const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules') +const newNodePath = + process.env.NODE_PATH ? + `${nodeModulesPath}${path.delimiter}${process.env.NODE_PATH}` + : nodeModulesPath + +const packageRoot = path.join(runfilesDir, '_main', 'app', 'electron-client') +const require = createRequire(import.meta.url) // Path to our custom ESM loader const loaderPath = path.join( @@ -28,18 +33,21 @@ const loaderPath = path.join( '_main', 'app', 'electron-client', - 'selective-symlink-loader.mjs' -); + 'selective-symlink-loader.mjs', +) // Playwright CLI path in runfiles -const playwrightCli = path.join( - nodeModulesPath, - '.aspect_rules_js', - 'playwright@1.55.1', - 'node_modules', - 'playwright', - 'cli.js' -); +let playwrightCli +try { + const playwrightPackageJson = require.resolve('playwright/package.json', { + paths: [packageRoot], + }) + playwrightCli = path.join(path.dirname(playwrightPackageJson), 'cli.js') +} catch (error) { + const details = error instanceof Error ? error.message : String(error) + console.error(`ERROR: Failed to resolve Playwright CLI: ${details}`) + process.exit(1) +} const child = spawn( process.execPath, @@ -48,10 +56,10 @@ const child = spawn( stdio: 'inherit', env: { ...process.env, NODE_PATH: newNodePath }, cwd: process.cwd(), - } -); + }, +) child.on('exit', (code, signal) => { - if (signal) process.kill(process.pid, signal); - else process.exit(code ?? 1); -}); + if (signal) process.kill(process.pid, signal) + else process.exit(code ?? 1) +}) diff --git a/app/electron-client/selective-symlink-loader-hooks.mjs b/app/electron-client/selective-symlink-loader-hooks.mjs index 47475eae0e25..ac06f21d8383 100644 --- a/app/electron-client/selective-symlink-loader-hooks.mjs +++ b/app/electron-client/selective-symlink-loader-hooks.mjs @@ -3,43 +3,43 @@ * This file is registered via module.register() from selective-symlink-loader.mjs */ -import { fileURLToPath, pathToFileURL } from 'node:url'; -import fs from 'node:fs'; -import path from 'node:path'; +import fs from 'node:fs' +import path from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' // Get the runfiles node_modules path from the main loader // This is set by the main loader before registering this hooks file -let runfilesNodeModules = null; +let runfilesNodeModules = null export function initialize(data) { // Get runfiles path from environment since globalThis isn't shared - const runfilesDir = process.env.JS_BINARY__RUNFILES; - runfilesNodeModules = runfilesDir - ? path.join(runfilesDir, '_main', 'node_modules') - : null; + const runfilesDir = process.env.JS_BINARY__RUNFILES + runfilesNodeModules = runfilesDir ? path.join(runfilesDir, '_main', 'node_modules') : null } /** * Helper function to rewrite bin/node_modules paths to runfiles */ function rewriteToRunfiles(resolved) { - if (!runfilesNodeModules || !resolved.includes('/bin/node_modules/')) { - return null; + if (!runfilesNodeModules) { + return null } - const match = resolved.match(/\/bin\/node_modules\/(.+)$/); - if (!match) { - return null; + const normalized = path.normalize(resolved) + const marker = `${path.sep}bin${path.sep}node_modules${path.sep}` + const markerIndex = normalized.indexOf(marker) + if (markerIndex === -1) { + return null } - const relativePath = match[1]; - const rewritten = path.join(runfilesNodeModules, relativePath); + const relativePath = normalized.slice(markerIndex + marker.length) + const rewritten = path.join(runfilesNodeModules, relativePath) try { - fs.accessSync(rewritten); - return rewritten; + fs.accessSync(rewritten) + return rewritten } catch { - return null; + return null } } @@ -47,16 +47,16 @@ function rewriteToRunfiles(resolved) { * ESM resolve hook - intercepts module resolution */ export async function resolve(specifier, context, nextResolve) { - const result = await nextResolve(specifier, context); + const result = await nextResolve(specifier, context) if (result.url && result.url.startsWith('file://')) { - const filePath = fileURLToPath(result.url); - const rewritten = rewriteToRunfiles(filePath); + const filePath = fileURLToPath(result.url) + const rewritten = rewriteToRunfiles(filePath) if (rewritten) { - const rewrittenUrl = pathToFileURL(rewritten).href; - return { ...result, url: rewrittenUrl }; + const rewrittenUrl = pathToFileURL(rewritten).href + return { ...result, url: rewrittenUrl } } } - return result; + return result } diff --git a/app/electron-client/selective-symlink-loader.mjs b/app/electron-client/selective-symlink-loader.mjs index 7635768d2460..4d9a0c741a42 100644 --- a/app/electron-client/selective-symlink-loader.mjs +++ b/app/electron-client/selective-symlink-loader.mjs @@ -17,7 +17,7 @@ * uses ESM resolution for ESM projects. */ -import { register } from 'node:module'; +import { register } from 'node:module' // Register ESM loader hooks from a separate file -register('./selective-symlink-loader-hooks.mjs', import.meta.url); +register('./selective-symlink-loader-hooks.mjs', import.meta.url) diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 02ab8848ca8a..993121d485ee 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -15,16 +15,31 @@ const LOADING_TIMEOUT = 10000 const TEXT = TEXTS.english const TEST_USER_FILE = path.join(import.meta.dirname, '../playwright/.auth/user.json') const POSSIBLE_ELECTRON_PATHS = [ + '../ide-dist/linux-unpacked/enso', + '../ide-dist/win-unpacked/Enso.exe', + '../ide-dist/mac/Enso.app/Contents/MacOS/Enso', + '../ide-dist/mac-arm64/Enso.app/Contents/MacOS/Enso', '../../../dist/ide/linux-unpacked/enso', '../../../dist/ide/win-unpacked/Enso.exe', '../../../dist/ide/mac/Enso.app/Contents/MacOS/Enso', '../../../dist/ide/mac-arm64/Enso.app/Contents/MacOS/Enso', ] -export const credentials: { readonly user: string; readonly password: string } = { - user: 'test@enso.org', - password: 'test', -} +export const credentials: { readonly user: string; readonly password: string } = await fs + .readFile(TEST_USER_FILE, { encoding: 'utf-8' }) + .then( + (contents) => JSON.parse(contents), + (error) => { + throw new Error(`Cannot read Test User credentials from '${TEST_USER_FILE}'.`, { + cause: error, + }) + }, + ) + .catch((error) => { + throw new Error(`Cannot parse Test User credentials from '${TEST_USER_FILE}'.`, { + cause: error, + }) + }) let _cachedElectronPath: string | undefined @@ -60,10 +75,17 @@ export async function getElectronExecutablePath(): Promise { */ export const electronFixtures = { // eslint-disable-next-line no-empty-pattern - testRunId: async function ({}, use: (value: string) => Promise, testInfo: { titlePath: string[] }) { + testRunId: async function ( + {}, + use: (value: string) => Promise, + testInfo: { titlePath: string[] }, + ) { await use(`${testInfo.titlePath.join('-')}-${Date.now()}`) }, - projectsDir: async function ({ testRunId }: { testRunId: string }, use: (value: string) => Promise) { + projectsDir: async function ( + { testRunId }: { testRunId: string }, + use: (value: string) => Promise, + ) { const projectsDir = path.join(os.tmpdir(), 'enso-test-projects', testRunId) await use(projectsDir) }, @@ -98,7 +120,10 @@ export const electronFixtures = { await app.close() }, page: async function ( - { app, viewport }: { app: ElectronApplication; viewport?: { width: number; height: number } | null }, + { + app, + viewport, + }: { app: ElectronApplication; viewport?: { width: number; height: number } | null }, use: (value: Page) => Promise, ) { const innerPage = await app.firstWindow() From ff9ddd11948c25f62416b57ef952852a238c5ac6 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 23 Jan 2026 18:08:00 +0400 Subject: [PATCH 03/11] More simplifications --- app/electron-client/BUILD.bazel | 5 +- app/electron-client/playwright-wrapper.mjs | 65 ---------------------- 2 files changed, 2 insertions(+), 68 deletions(-) delete mode 100644 app/electron-client/playwright-wrapper.mjs diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index 4b4cbe0c49c7..1e4eaebb6148 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -1,6 +1,5 @@ load("@aspect_bazel_lib//lib:write_source_files.bzl", "write_source_files") load("@aspect_rules_esbuild//esbuild:defs.bzl", "esbuild") -load("@aspect_rules_js//js:defs.bzl", "js_test") load("@aspect_rules_js//npm:defs.bzl", "npm_package") load("@aspect_rules_ts//ts:defs.bzl", "ts_config", "ts_project") load("@npm//:defs.bzl", "npm_link_all_packages", "npm_link_targets") @@ -169,9 +168,8 @@ write_source_files( visibility = ["//visibility:public"], ) -js_test( +playwright_bin.playwright_test( name = "test", - entry_point = "playwright-wrapper.mjs", data = npm_link_targets() + glob(["tests/*.ts"]) + glob(["playwright/.auth/user.json"], allow_empty = True) + [ ":dist", "playwright.config.ts", @@ -183,5 +181,6 @@ js_test( chdir = package_name(), visibility = ["//visibility:public"], log_level = "debug", + node_options = ["--import=./selective-symlink-loader.mjs"], tags = ["local"], ) diff --git a/app/electron-client/playwright-wrapper.mjs b/app/electron-client/playwright-wrapper.mjs deleted file mode 100644 index 815b8822ff5c..000000000000 --- a/app/electron-client/playwright-wrapper.mjs +++ /dev/null @@ -1,65 +0,0 @@ -#!/usr/bin/env node -/** - * Custom Playwright test runner wrapper for Bazel. - * Uses a custom ESM loader to solve dual-module instance issues. - * - * Problem: Bazel creates two node_modules paths (bin/ and runfiles/). - * Solution: Use --import flag to load a custom ESM resolver that rewrites - * bin/ paths back to runfiles/ for source files. - */ -import { spawn } from 'node:child_process' -import { createRequire } from 'node:module' -import path from 'node:path' - -const runfilesDir = process.env.JS_BINARY__RUNFILES -if (!runfilesDir) { - console.error('ERROR: JS_BINARY__RUNFILES not set. Run via Bazel.') - process.exit(1) -} - -// Set NODE_PATH to runfiles node_modules -const nodeModulesPath = path.join(runfilesDir, '_main', 'node_modules') -const newNodePath = - process.env.NODE_PATH ? - `${nodeModulesPath}${path.delimiter}${process.env.NODE_PATH}` - : nodeModulesPath - -const packageRoot = path.join(runfilesDir, '_main', 'app', 'electron-client') -const require = createRequire(import.meta.url) - -// Path to our custom ESM loader -const loaderPath = path.join( - runfilesDir, - '_main', - 'app', - 'electron-client', - 'selective-symlink-loader.mjs', -) - -// Playwright CLI path in runfiles -let playwrightCli -try { - const playwrightPackageJson = require.resolve('playwright/package.json', { - paths: [packageRoot], - }) - playwrightCli = path.join(path.dirname(playwrightPackageJson), 'cli.js') -} catch (error) { - const details = error instanceof Error ? error.message : String(error) - console.error(`ERROR: Failed to resolve Playwright CLI: ${details}`) - process.exit(1) -} - -const child = spawn( - process.execPath, - ['--import', loaderPath, playwrightCli, ...process.argv.slice(2)], - { - stdio: 'inherit', - env: { ...process.env, NODE_PATH: newNodePath }, - cwd: process.cwd(), - }, -) - -child.on('exit', (code, signal) => { - if (signal) process.kill(process.pid, signal) - else process.exit(code ?? 1) -}) From 67eb5f3d8f4a49f75676c8d168d8ea6e15b67cae Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Fri, 23 Jan 2026 18:36:07 +0400 Subject: [PATCH 04/11] Collect test reports and some optional flags for logs --- app/electron-client/playwright.config.ts | 12 ++++++ app/electron-client/tests/electronTest.ts | 47 ++++++++++++++++++----- 2 files changed, 49 insertions(+), 10 deletions(-) diff --git a/app/electron-client/playwright.config.ts b/app/electron-client/playwright.config.ts index 675f6804f0dd..03d2d8042a7f 100644 --- a/app/electron-client/playwright.config.ts +++ b/app/electron-client/playwright.config.ts @@ -1,6 +1,9 @@ /** @file Playwright browser testing configuration. */ +import path from 'node:path' import { defineConfig } from 'playwright/test' +const outputDir = process.env.TEST_UNDECLARED_OUTPUTS_DIR ?? 'test-results' + export default defineConfig({ testDir: './tests', // Headless tests are run via vitest, not playwright, so we ignore them here. @@ -9,6 +12,13 @@ export default defineConfig({ workers: 1, timeout: 180000, reportSlowTests: { max: 5, threshold: 60000 }, + outputDir, + reporter: [ + ['list'], + ['html', { outputFolder: path.join(outputDir, 'html-report'), open: 'never' }], + ['junit', { outputFile: path.join(outputDir, 'junit.xml') }], + ['json', { outputFile: path.join(outputDir, 'report.json') }], + ], expect: { timeout: 30000, toHaveScreenshot: { threshold: 0 }, @@ -16,5 +26,7 @@ export default defineConfig({ use: { actionTimeout: 5000, viewport: { width: 1380, height: 900 }, + screenshot: process.env.ENSO_PW_SCREENSHOTS ? 'only-on-failure' : 'off', + video: process.env.ENSO_PW_VIDEO ? 'retain-on-failure' : 'off', }, }) diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 993121d485ee..894619289346 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -9,11 +9,14 @@ import { type ElectronApplication, type Locator, type Page, + type TestInfo, } from 'playwright/test' const LOADING_TIMEOUT = 10000 const TEXT = TEXTS.english const TEST_USER_FILE = path.join(import.meta.dirname, '../playwright/.auth/user.json') +const LOG_DIAGNOSTICS = + process.env.ENSO_PW_LOG_CONSOLE === '1' || process.env.ENSO_PW_LOG_CONSOLE === 'true' const POSSIBLE_ELECTRON_PATHS = [ '../ide-dist/linux-unpacked/enso', '../ide-dist/win-unpacked/Enso.exe', @@ -75,11 +78,7 @@ export async function getElectronExecutablePath(): Promise { */ export const electronFixtures = { // eslint-disable-next-line no-empty-pattern - testRunId: async function ( - {}, - use: (value: string) => Promise, - testInfo: { titlePath: string[] }, - ) { + testRunId: async function ({}, use: (value: string) => Promise, testInfo: TestInfo) { await use(`${testInfo.titlePath.join('-')}-${Date.now()}`) }, projectsDir: async function ( @@ -92,8 +91,9 @@ export const electronFixtures = { /** Setup for all tests: Create an electron-based app instance. */ app: async function ( - { projectsDir, testRunId }: { projectsDir: string; testRunId: string }, + { projectsDir }: { projectsDir: string }, use: (value: ElectronApplication) => Promise, + testInfo: TestInfo, ) { const args = process.env.ENSO_TEST_APP_ARGS?.split(',') ?? [] const executablePath = await getElectronExecutablePath() @@ -114,10 +114,21 @@ export const electronFixtures = { ;(await app.firstWindow()).evaluate((password) => { ;(window as any).passwordOverride = password }, credentials.password) - await app.context().tracing.start({ screenshots: true, snapshots: true, sources: true }) - await use(app) - await app.context().tracing.stop({ path: `test-traces/${testRunId}.zip` }) - await app.close() + const context = app.context() + const tracePath = testInfo.outputPath('trace.zip') + await context.tracing.start({ screenshots: true, snapshots: true, sources: true }) + try { + await use(app) + } finally { + const shouldSaveTrace = testInfo.status !== testInfo.expectedStatus + if (shouldSaveTrace) { + await context.tracing.stop({ path: tracePath }) + await testInfo.attach('trace', { path: tracePath, contentType: 'application/zip' }) + } else { + await context.tracing.stop() + } + await app.close() + } }, page: async function ( { @@ -127,6 +138,22 @@ export const electronFixtures = { use: (value: Page) => Promise, ) { const innerPage = await app.firstWindow() + if (LOG_DIAGNOSTICS) { + innerPage.on('console', (message) => { + const location = message.location() + const locationText = + location.url ? ` (${location.url}:${location.lineNumber}:${location.columnNumber})` : '' + console.log(`[pw:console:${message.type()}] ${message.text()}${locationText}`) + }) + innerPage.on('pageerror', (error) => { + console.log(`[pw:pageerror] ${error.message}`) + }) + innerPage.on('requestfailed', (request) => { + const failure = request.failure() + const message = failure?.errorText ? ` ${failure.errorText}` : '' + console.log(`[pw:requestfailed] ${request.method()} ${request.url()}${message}`) + }) + } if (viewport) innerPage.setViewportSize(viewport) await use(innerPage) }, From ab50e938995fa276a2cf0d7cd31369d9f37e3eb0 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 26 Jan 2026 12:23:26 +0100 Subject: [PATCH 05/11] Play a little --- app/electron-client/BUILD.bazel | 3 +++ app/electron-client/tests/electronTest.ts | 3 +++ 2 files changed, 6 insertions(+) diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index 1e4eaebb6148..5b8346c24163 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -177,6 +177,9 @@ playwright_bin.playwright_test( "selective-symlink-loader.mjs", "selective-symlink-loader-hooks.mjs", ], + env = { + "ENSO_EXEC_PATH": "../../../../../../../../../../$(location :dist)/enso-linux-x86_64-0.0.0-dev.AppImage" + }, args = ["test"], chdir = package_name(), visibility = ["//visibility:public"], diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 894619289346..09a38d550378 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -47,6 +47,9 @@ export const credentials: { readonly user: string; readonly password: string } = let _cachedElectronPath: string | undefined export async function getElectronExecutablePath(): Promise { + if (process.env.ENSO_EXEC_PATH) { + return process.env.ENSO_EXEC_PATH + } if (_cachedElectronPath !== undefined) return _cachedElectronPath try { const promises = POSSIBLE_ELECTRON_PATHS.map((p) => path.resolve(import.meta.dirname, p)).map( From cf02da246dcf59d51e280065a614d1878070d8c3 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 26 Jan 2026 15:34:24 +0100 Subject: [PATCH 06/11] playing with paths --- app/electron-client/BUILD.bazel | 6 +++--- app/electron-client/tests/electronTest.ts | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index 5b8346c24163..a210204dca15 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -178,12 +178,12 @@ playwright_bin.playwright_test( "selective-symlink-loader-hooks.mjs", ], env = { - "ENSO_EXEC_PATH": "../../../../../../../../../../$(location :dist)/enso-linux-x86_64-0.0.0-dev.AppImage" + "ENSO_EXEC_PATH": "$(rootpath :dist)/enso-linux-x86_64-0.0.0-dev.AppImage" }, args = ["test"], - chdir = package_name(), + #chdir = package_name(), visibility = ["//visibility:public"], log_level = "debug", - node_options = ["--import=./selective-symlink-loader.mjs"], + node_options = ["--import=./$(rootpath selective-symlink-loader.mjs)"], tags = ["local"], ) diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 09a38d550378..96d3da263c78 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -28,6 +28,8 @@ const POSSIBLE_ELECTRON_PATHS = [ '../../../dist/ide/mac-arm64/Enso.app/Contents/MacOS/Enso', ] +console.debug('PWD', process.env.PWD) + export const credentials: { readonly user: string; readonly password: string } = await fs .readFile(TEST_USER_FILE, { encoding: 'utf-8' }) .then( From 14a14ca2213c388ded6d74f7725ae1eda739a942 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Fri, 30 Jan 2026 15:40:21 +0100 Subject: [PATCH 07/11] Some touches --- app/electron-client/BUILD.bazel | 5 ++- .../electron-builder-config.cjs | 2 +- app/electron-client/playwright.config.ts | 10 ++---- app/electron-client/tests/electronTest.ts | 32 ++++++++----------- 4 files changed, 20 insertions(+), 29 deletions(-) diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index a210204dca15..eb521cbc969e 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -178,10 +178,9 @@ playwright_bin.playwright_test( "selective-symlink-loader-hooks.mjs", ], env = { - "ENSO_EXEC_PATH": "$(rootpath :dist)/enso-linux-x86_64-0.0.0-dev.AppImage" + "ENSO_EXEC_PATH": "$(rootpath :dist)" }, - args = ["test"], - #chdir = package_name(), + args = ["test", "--config=$(rootpath playwright.config.ts)"], visibility = ["//visibility:public"], log_level = "debug", node_options = ["--import=./$(rootpath selective-symlink-loader.mjs)"], diff --git a/app/electron-client/electron-builder-config.cjs b/app/electron-client/electron-builder-config.cjs index 875f549663ef..8e3c00bcc902 100644 --- a/app/electron-client/electron-builder-config.cjs +++ b/app/electron-client/electron-builder-config.cjs @@ -69,7 +69,7 @@ async function patchAppImage(context) { # TODO[ib]: quick hack to resolve java binary at runtime on Linux SCRIPT_DIR="$( cd "$( dirname "\${BASH_SOURCE[0]}" )" && pwd )" - export PATH="$PATH:$SCRIPT_DIR/resources/enso/runtime/graalvm-ce-java24.0.1-24.2.0/bin" + export PATH="$SCRIPT_DIR/resources/enso/runtime/graalvm-ce-java24.0.1-24.2.0/bin:$PATH" exec "$SCRIPT_DIR/${executableName}.bin" --no-sandbox "$@" ` try { diff --git a/app/electron-client/playwright.config.ts b/app/electron-client/playwright.config.ts index 03d2d8042a7f..40aecd08b423 100644 --- a/app/electron-client/playwright.config.ts +++ b/app/electron-client/playwright.config.ts @@ -1,9 +1,6 @@ /** @file Playwright browser testing configuration. */ -import path from 'node:path' import { defineConfig } from 'playwright/test' -const outputDir = process.env.TEST_UNDECLARED_OUTPUTS_DIR ?? 'test-results' - export default defineConfig({ testDir: './tests', // Headless tests are run via vitest, not playwright, so we ignore them here. @@ -12,12 +9,11 @@ export default defineConfig({ workers: 1, timeout: 180000, reportSlowTests: { max: 5, threshold: 60000 }, - outputDir, reporter: [ ['list'], - ['html', { outputFolder: path.join(outputDir, 'html-report'), open: 'never' }], - ['junit', { outputFile: path.join(outputDir, 'junit.xml') }], - ['json', { outputFile: path.join(outputDir, 'report.json') }], + ['html', { outputFolder: 'html-report', open: 'never' }], + ['junit', { outputFile: 'junit.xml' }], + ['json', { outputFile: 'report.json' }], ], expect: { timeout: 30000, diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index 96d3da263c78..fbe0aaf10d30 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -17,18 +17,18 @@ const TEXT = TEXTS.english const TEST_USER_FILE = path.join(import.meta.dirname, '../playwright/.auth/user.json') const LOG_DIAGNOSTICS = process.env.ENSO_PW_LOG_CONSOLE === '1' || process.env.ENSO_PW_LOG_CONSOLE === 'true' -const POSSIBLE_ELECTRON_PATHS = [ - '../ide-dist/linux-unpacked/enso', - '../ide-dist/win-unpacked/Enso.exe', - '../ide-dist/mac/Enso.app/Contents/MacOS/Enso', - '../ide-dist/mac-arm64/Enso.app/Contents/MacOS/Enso', - '../../../dist/ide/linux-unpacked/enso', - '../../../dist/ide/win-unpacked/Enso.exe', - '../../../dist/ide/mac/Enso.app/Contents/MacOS/Enso', - '../../../dist/ide/mac-arm64/Enso.app/Contents/MacOS/Enso', +const POSSIBLE_ELECTRON_DIRS = [ + ...(process.env.ENSO_EXEC_PATH ? [process.env.ENSO_EXEC_PATH] : []), + '../ide-dist/', + '../../../dist/ide/', ] -console.debug('PWD', process.env.PWD) +const POSSIBLE_ELECTRON_PATHS = POSSIBLE_ELECTRON_DIRS.flatMap((dir) => [ + path.join(dir, 'linux-unpacked/enso'), + path.join(dir, 'win-unpacked/Enso.exe'), + path.join(dir, 'mac/Enso.app/Contents/MacOS/Enso'), + path.join(dir, 'mac-arm64/Enso.app/Contents/MacOS/Enso'), +]) export const credentials: { readonly user: string; readonly password: string } = await fs .readFile(TEST_USER_FILE, { encoding: 'utf-8' }) @@ -46,19 +46,16 @@ export const credentials: { readonly user: string; readonly password: string } = }) }) -let _cachedElectronPath: string | undefined +let cachedElectronPath: string | undefined export async function getElectronExecutablePath(): Promise { - if (process.env.ENSO_EXEC_PATH) { - return process.env.ENSO_EXEC_PATH - } - if (_cachedElectronPath !== undefined) return _cachedElectronPath + if (cachedElectronPath !== undefined) return cachedElectronPath try { const promises = POSSIBLE_ELECTRON_PATHS.map((p) => path.resolve(import.meta.dirname, p)).map( (p) => fs.access(p, fs.constants.X_OK).then(() => p), ) - _cachedElectronPath = await Promise.any(promises) - return _cachedElectronPath + cachedElectronPath = await Promise.any(promises) + return cachedElectronPath } catch { return undefined } @@ -68,7 +65,6 @@ export async function getElectronExecutablePath(): Promise { * Custom test fixtures for Electron tests. * Spec files should import `test` directly from `playwright/test` and extend it * with these fixtures to avoid dual module instance issues with Bazel. - * * @example * ```ts * import { test as base, expect } from 'playwright/test' From 12dd56783a62a44ade29685fd90ae1539c72a9d7 Mon Sep 17 00:00:00 2001 From: Adam Obuchowicz Date: Mon, 2 Feb 2026 13:42:11 +0100 Subject: [PATCH 08/11] WIP --- app/electron-client/BUILD.bazel | 4 ++-- app/electron-client/electron-builder-config.cjs | 12 ++++++------ app/gui/tsconfig.app.json | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index 8fed2c81928b..1cae64c1f41e 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -122,8 +122,8 @@ electron_builder_bin.electron_builder( ":electron-builder-config.cjs", ":selected_bundle", ":selected_gui_dist", - "//:sbt_build_engine_distribution", - "//:sbt_build_small_jdk", + "//:sbt_build_native_engine_distribution", + # "//:sbt_build_small_jdk", ], args = [ "build", diff --git a/app/electron-client/electron-builder-config.cjs b/app/electron-client/electron-builder-config.cjs index ac9772d8b75a..e191868ed33c 100644 --- a/app/electron-client/electron-builder-config.cjs +++ b/app/electron-client/electron-builder-config.cjs @@ -16,7 +16,7 @@ function engineDistributionSource(version, platform = process.platform, arch = p const normalizedPlatform = platformMap[platform] ?? platform const normalizedArch = archMapByPlatform[platform]?.[arch] ?? arch - return `../../built-distribution/enso-engine-${version}-${normalizedPlatform}-${normalizedArch}/enso-${version}/` + return `../../built-distribution-native/enso-engine-${version}-${normalizedPlatform}-${normalizedArch}/enso-${version}/` } function engineDistributionTarget(version) { @@ -70,7 +70,7 @@ async function patchAppImage(context) { # TODO[ib]: quick hack to resolve java binary at runtime on Linux SCRIPT_DIR="$( cd "$( dirname "\${BASH_SOURCE[0]}" )" && pwd )" - export PATH="$SCRIPT_DIR/resources/enso/runtime/graalvm-ce-java24.0.1-24.2.0/bin:$PATH" + # export PATH="$SCRIPT_DIR/resources/enso/runtime/graalvm-ce-java24.0.1-24.2.0/bin:$PATH" exec "$SCRIPT_DIR/${executableName}.bin" --no-sandbox "$@" ` try { @@ -147,10 +147,10 @@ module.exports = { from: engineDistributionSource('0.0.0-dev'), to: engineDistributionTarget('0.0.0-dev'), }, - { - from: '../../built-small-jdk/', - to: 'enso/runtime/graalvm-ce-java24.0.1-24.2.0', - }, + // { + // from: '../../built-small-jdk/', + // to: 'enso/runtime/graalvm-ce-java24.0.1-24.2.0', + // }, ], fileAssociations: [ { diff --git a/app/gui/tsconfig.app.json b/app/gui/tsconfig.app.json index 2b38b04d5e25..5d66a4448a71 100644 --- a/app/gui/tsconfig.app.json +++ b/app/gui/tsconfig.app.json @@ -841,6 +841,7 @@ "./src/project-view/util/fetchTimeout.ts", "./src/project-view/util/fileFilter.ts", "./src/project-view/util/getIconName.ts", + "./src/project-view/util/iconMetadata/iconName.ts", "./src/project-view/util/icons.ts", "./src/project-view/util/link.ts", "./src/project-view/util/measurement.ts", @@ -936,7 +937,6 @@ "./src/utils/queryClient.ts", "./src/utils/reactivity.ts", "./src/utils/style/tabBar.ts", - "./src/utils/zustand.ts", - "./src/project-view/util/iconMetadata/iconName.ts" + "./src/utils/zustand.ts" ] } From f3440caeb843a86cd77d66238a39f579644ec308 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Thu, 19 Mar 2026 15:27:48 +0400 Subject: [PATCH 09/11] WIP working on playwright tests --- app/electron-client/BUILD.bazel | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index b8438522fba5..aa25342af977 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -123,12 +123,10 @@ electron_builder_bin.electron_builder( srcs = npm_link_targets() + [ "assets/icons/icon.icns", "package.json", - ":electron-builder-config.cjs", - ":selected_bundle", - ":selected_gui_dist", ":bundle_stamped", ":electron_builder_config_stamped", ":enso_bundle_marker", + "//:dependencies_scala_bin", "//:sbt_build_native_engine_distribution", "//:sbt_build_small_jdk", "//app/gui:dist_stamped", From 402fd5b5bcbfe46027de325525d71509f042278b Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Mon, 23 Mar 2026 15:37:06 +0400 Subject: [PATCH 10/11] Working tests --- BUILD.bazel | 2 - .../services/ProjectManager/ProjectManager.ts | 26 +++- app/electron-client/BUILD.bazel | 3 +- app/electron-client/src/globals.d.ts | 4 + app/electron-client/src/index.ts | 3 +- app/electron-client/tests/electronTest.ts | 143 +++++++++++++++--- .../tests/gettingStarted.spec.ts | 96 ++++++------ app/gui/tsconfig.app.json | 2 +- .../src/handler/jsonrpc.ts | 22 ++- .../src/projectService/ensoRunner.ts | 24 ++- .../src/projectService/index.ts | 13 +- tools/enso4igv/.project | 28 ++++ .../org.eclipse.core.resources.prefs | 2 + .../.settings/org.eclipse.m2e.core.prefs | 4 + 14 files changed, 289 insertions(+), 83 deletions(-) create mode 100644 tools/enso4igv/.project create mode 100644 tools/enso4igv/.settings/org.eclipse.core.resources.prefs create mode 100644 tools/enso4igv/.settings/org.eclipse.m2e.core.prefs diff --git a/BUILD.bazel b/BUILD.bazel index a2a8e6e46eb4..1bd59480c211 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -94,8 +94,6 @@ ENGINE_DIST_GLOB_EXCLUDES = [ "**/.metals/**", "**/.enso/**", "**/.enso-sources*", - # Generated/downloaded resources should not be treated as inputs. - "lib/scala/pkg/src/main/resources/**", # Binary test fixtures (often ignored) should not affect distribution builds. "lib/scala/runtime-version-manager/src/test/resources/**", ] diff --git a/app/common/src/services/ProjectManager/ProjectManager.ts b/app/common/src/services/ProjectManager/ProjectManager.ts index 1b699042b829..56e815590157 100644 --- a/app/common/src/services/ProjectManager/ProjectManager.ts +++ b/app/common/src/services/ProjectManager/ProjectManager.ts @@ -411,7 +411,31 @@ export class ProjectManager { if ('result' in json) { return json.result } else { - throw new Error(json.error.message) + const detail = + json.error.data != null ? extractProjectServiceErrorDetail(json.error.data) : undefined + console.error(`[ProjectManager] ${name} failed`, { + requestBody: body, + responseStatus: response.status, + error: json.error, + }) + throw new Error(detail ? `${json.error.message}: ${detail}` : json.error.message) + } + } +} + +function extractProjectServiceErrorDetail(data: unknown): string | undefined { + if (data == null) { + return undefined + } + if (typeof data === 'string') { + return data + } + if (typeof data === 'object') { + const maybeMessage = 'message' in data ? data.message : undefined + if (typeof maybeMessage === 'string') { + const maybeCause = 'cause' in data ? extractProjectServiceErrorDetail(data.cause) : undefined + return maybeCause ? `${maybeMessage}; cause: ${maybeCause}` : maybeMessage } } + return undefined } diff --git a/app/electron-client/BUILD.bazel b/app/electron-client/BUILD.bazel index aa25342af977..cc7c6bc16ae1 100644 --- a/app/electron-client/BUILD.bazel +++ b/app/electron-client/BUILD.bazel @@ -237,7 +237,8 @@ write_source_files( playwright_bin.playwright_test( name = "test", - data = npm_link_targets() + glob(["tests/*.ts"]) + glob(["playwright/.auth/user.json"], allow_empty = True) + [ + data = npm_link_targets() + glob(["tests/*.ts"]) + [ + "playwright/.auth/user.json", ":dist", "playwright.config.ts", "tests/package.json", diff --git a/app/electron-client/src/globals.d.ts b/app/electron-client/src/globals.d.ts index 079edeac2b19..a1b12cfaaf64 100644 --- a/app/electron-client/src/globals.d.ts +++ b/app/electron-client/src/globals.d.ts @@ -57,6 +57,10 @@ declare global { readonly ENSO_TEST_PROJECTS_DIR?: string readonly ENSO_TEST_APP_ARGS?: string readonly ENSO_TEST_USER?: string + readonly ENSO_TEST_PASS?: string + readonly ENSO_TEST_PARTITION?: string + readonly ENSO_EXEC_PATH?: string + readonly JS_BINARY__RUNFILES?: string ENSO_TEST_EXEC_PATH?: string // === Electron watch script variables === diff --git a/app/electron-client/src/index.ts b/app/electron-client/src/index.ts index ee1517f692cd..3e21bfbd40a9 100644 --- a/app/electron-client/src/index.ts +++ b/app/electron-client/src/index.ts @@ -346,12 +346,13 @@ class App { const guiConfig = await loadGuiConfig() const encodedGuiConfig = Buffer.from(JSON.stringify(guiConfig), 'utf8').toString('base64') + const testPartition = process.env.ENSO_TEST ? (process.env.ENSO_TEST_PARTITION ?? 'test') : null const webPreferences: WebPreferences = { preload: joinPath(appPath(this.electron), 'preload.mjs'), sandbox: true, spellcheck: false, additionalArguments: [`--enso-gui-config=${encodedGuiConfig}`], - ...(process.env.ENSO_TEST ? { partition: 'test' } : {}), + ...(testPartition ? { partition: testPartition } : {}), } const windowPreferences: BrowserWindowConstructorOptions = { webPreferences, diff --git a/app/electron-client/tests/electronTest.ts b/app/electron-client/tests/electronTest.ts index dac5d2a64276..68033305e8e7 100644 --- a/app/electron-client/tests/electronTest.ts +++ b/app/electron-client/tests/electronTest.ts @@ -14,14 +14,46 @@ import { const LOADING_TIMEOUT = 10000 const TEXT = TEXTS.english -const TEST_USER_FILE = path.join(import.meta.dirname, '../playwright/.auth/user.json') const LOG_DIAGNOSTICS = process.env.ENSO_PW_LOG_CONSOLE === '1' || process.env.ENSO_PW_LOG_CONSOLE === 'true' -const POSSIBLE_ELECTRON_DIRS = [ - ...(process.env.ENSO_EXEC_PATH ? [process.env.ENSO_EXEC_PATH] : []), - '../ide-dist/', - '../../../dist/ide/', -] +const RUNFILES_WORKSPACE_ROOT = + process.env.JS_BINARY__RUNFILES ? path.join(process.env.JS_BINARY__RUNFILES, '_main') : undefined + +function uniquePaths(paths: readonly (string | undefined)[]): string[] { + return [ + ...new Set(paths.filter((value): value is string => value != null).map((p) => path.resolve(p))), + ] +} + +function workspacePathCandidates(relativeOrAbsolutePath: string): string[] { + if (path.isAbsolute(relativeOrAbsolutePath)) { + return [relativeOrAbsolutePath] + } + return uniquePaths([ + path.resolve(process.cwd(), relativeOrAbsolutePath), + RUNFILES_WORKSPACE_ROOT ? + path.join(RUNFILES_WORKSPACE_ROOT, relativeOrAbsolutePath) + : undefined, + ]) +} + +function logCandidatePaths(label: string, candidates: readonly string[]) { + if (LOG_DIAGNOSTICS) { + console.log(`[pw:${label}] ${JSON.stringify(candidates)}`) + } +} + +const TEST_USER_FILE_CANDIDATES = uniquePaths([ + path.join(import.meta.dirname, '../playwright/.auth/user.json'), + path.join(process.cwd(), 'playwright/.auth/user.json'), + ...workspacePathCandidates('app/electron-client/playwright/.auth/user.json'), +]) + +const POSSIBLE_ELECTRON_DIRS = uniquePaths([ + ...(process.env.ENSO_EXEC_PATH ? workspacePathCandidates(process.env.ENSO_EXEC_PATH) : []), + path.join(import.meta.dirname, '../ide-dist'), + path.join(import.meta.dirname, '../../../dist/ide'), +]) const POSSIBLE_ELECTRON_PATHS = POSSIBLE_ELECTRON_DIRS.flatMap((dir) => [ path.join(dir, 'linux-unpacked/enso'), @@ -30,31 +62,58 @@ const POSSIBLE_ELECTRON_PATHS = POSSIBLE_ELECTRON_DIRS.flatMap((dir) => [ path.join(dir, 'mac-arm64/Enso.app/Contents/MacOS/Enso'), ]) -export const credentials: { readonly user: string; readonly password: string } = await fs - .readFile(TEST_USER_FILE, { encoding: 'utf-8' }) - .then( - (contents) => JSON.parse(contents), - (error) => { - throw new Error(`Cannot read Test User credentials from '${TEST_USER_FILE}'.`, { - cause: error, - }) - }, +logCandidatePaths('credentials', TEST_USER_FILE_CANDIDATES) +logCandidatePaths('executables', POSSIBLE_ELECTRON_PATHS) + +type TestCredentials = { readonly user: string; readonly password: string } + +async function readCredentialsFile(filePath: string): Promise { + const contents = await fs.readFile(filePath, { encoding: 'utf-8' }) + const parsed = JSON.parse(contents) as Partial + if (!parsed.user || !parsed.password) { + throw new Error(`Missing 'user' or 'password' key in '${filePath}'.`) + } + return { user: parsed.user, password: parsed.password } +} + +async function loadCredentials(): Promise { + let lastError: unknown = undefined + for (const candidate of TEST_USER_FILE_CANDIDATES) { + try { + return await readCredentialsFile(candidate) + } catch (error) { + lastError = error + const code = typeof error === 'object' && error != null && 'code' in error ? error.code : null + if (code !== 'ENOENT') { + console.warn(`[pw:credentials] Failed to read '${candidate}':`, error) + } + } + } + + if (process.env.ENSO_TEST_USER && process.env.ENSO_TEST_PASS) { + return { user: process.env.ENSO_TEST_USER, password: process.env.ENSO_TEST_PASS } + } + + throw new Error( + `Cannot load test credentials. Set ENSO_TEST_USER and ENSO_TEST_PASS, or create one of: ${TEST_USER_FILE_CANDIDATES.join(', ')}`, + { cause: lastError }, ) - .catch((error) => { - throw new Error(`Cannot parse Test User credentials from '${TEST_USER_FILE}'.`, { - cause: error, - }) - }) +} + +export const credentials = await loadCredentials() let cachedElectronPath: string | undefined export async function getElectronExecutablePath(): Promise { if (cachedElectronPath !== undefined) return cachedElectronPath try { - const promises = POSSIBLE_ELECTRON_PATHS.map((p) => path.resolve(import.meta.dirname, p)).map( - (p) => fs.access(p, fs.constants.X_OK).then(() => p), + const promises = POSSIBLE_ELECTRON_PATHS.map((p) => + fs.access(p, fs.constants.X_OK).then(() => p), ) cachedElectronPath = await Promise.any(promises) + if (LOG_DIAGNOSTICS) { + console.log(`[pw:executable] Using '${cachedElectronPath}'.`) + } return cachedElectronPath } catch { return undefined @@ -87,12 +146,13 @@ export const electronFixtures = { use: (value: string) => Promise, ) { const projectsDir = path.join(os.tmpdir(), 'enso-test-projects', testRunId) + await fs.mkdir(projectsDir, { recursive: true }) await use(projectsDir) }, /** Setup for all tests: Create an electron-based app instance. */ app: async function ( - { projectsDir }: { projectsDir: string }, + { projectsDir, testRunId }: { projectsDir: string; testRunId: string }, use: (value: ElectronApplication) => Promise, testInfo: TestInfo, ) { @@ -106,7 +166,9 @@ export const electronFixtures = { args, env: { ...process.env, + HOME: process.env.HOME ?? os.homedir(), ENSO_TEST: 'true', + ENSO_TEST_PARTITION: `enso-test-${testRunId}`, ENSO_TEST_PROJECTS_DIR: projectsDir.replace(/\\/g, '/'), }, }) @@ -248,6 +310,13 @@ export async function visualizeData(page: Page) { await showViz.click({ timeout: 5000 }) } +/** Click a locator even when Playwright cannot bring it into the viewport. */ +export async function clickWithoutViewportConstraints(locator: Locator) { + const target = locator.first() + await expect(target).toBeVisible() + await target.dispatchEvent('click', { bubbles: true, cancelable: true }) +} + /** * Open new component browser refefencing the last created component */ @@ -262,7 +331,29 @@ export async function createNewComponent(page: Page) { * Open new component browser based on the name of referenced parent component */ export async function openComponentBrowser(page: Page, parentComponent: string) { - await page.getByText(parentComponent, { exact: true }).click() + await clickWithoutViewportConstraints(page.getByText(parentComponent, { exact: true })) + await page.keyboard.press('Enter') +} + +/** Select a component browser entry without relying on viewport-based mouse input. */ +export async function selectComponentEntry(page: Page, name: string | RegExp) { + await clickWithoutViewportConstraints(page.locator('.ComponentEntry', { hasText: name })) +} + +/** Open component browser without relying on viewport-bound right click. */ +export async function openComponentBrowserFromGraphNode( + page: Page, + parentComponent: string, + occurrence = 0, +) { + const target = page.getByText(parentComponent, { exact: true }).nth(occurrence) + await expect(target).toBeVisible() + await target.dispatchEvent('contextmenu', { + button: 2, + buttons: 2, + bubbles: true, + cancelable: true, + }) await page.keyboard.press('Enter') } @@ -303,7 +394,9 @@ export async function waitForDownload(pathToFile: string): Promise { /** Open drop-down menu in WidgetSelection with given label. */ export function openDropdownInWidget(page: Page, label: string) { - return page.locator('.WidgetSelection', { hasText: new RegExp(`^${label}$`) }).click() + return clickWithoutViewportConstraints( + page.locator('.WidgetSelection', { hasText: new RegExp(`^${label}$`) }), + ) } /** Find and click + button in an empty Vector Widget inside provided locator. */ diff --git a/app/electron-client/tests/gettingStarted.spec.ts b/app/electron-client/tests/gettingStarted.spec.ts index c745ce7e8a57..2c6ad9aac7bc 100644 --- a/app/electron-client/tests/gettingStarted.spec.ts +++ b/app/electron-client/tests/gettingStarted.spec.ts @@ -4,13 +4,16 @@ import path from 'path' import { test as base, expect } from 'playwright/test' import { addFirstElementToWidgetVector, + clickWithoutViewportConstraints, closeWelcome, createNewProject, electronFixtures, fillWidgetText, loginAsTestUser, openComponentBrowser, + openComponentBrowserFromGraphNode, openDropdownInWidget, + selectComponentEntry, visualizeData, waitForDownload, } from './electronTest' @@ -30,9 +33,7 @@ test('Exercise 1', async ({ page, projectsDir }) => { await expect(addComponent).toBeVisible() await addComponent.click() - const dataReadEntry = page.locator('.ComponentEntry', { hasText: /^Data\.read$/ }) - await expect(dataReadEntry).toBeVisible() - await dataReadEntry.click() + await selectComponentEntry(page, /^Data\.read$/) // Create relative path to file const filePath = path.join(projectsDir, 'Samples', 'Data', 'sample_bank_data.xlsx') @@ -62,7 +63,7 @@ test('Exercise 1', async ({ page, projectsDir }) => { await test.step('Objective 2: Filter Data to find “exception” records', async () => { // Adding set component await openComponentBrowser(page, 'readquery‘Sheet1’') - await page.locator('.ComponentEntry', { hasText: 'set' }).click() + await selectComponentEntry(page, 'set') // Playwright seems to not always fire proper pointerleave event // We must do that ourselves, to hide circular menu of the node. @@ -95,7 +96,7 @@ test('Exercise 1', async ({ page, projectsDir }) => { // Adding filter component await openComponentBrowser(page, 'set') - await page.locator('.ComponentEntry', { hasText: 'filter' }).click() + await selectComponentEntry(page, 'filter') await openDropdownInWidget(page, 'column') // Click with the assurance of component being in vision @@ -133,7 +134,7 @@ test('Exercise 1', async ({ page, projectsDir }) => { await page.getByText('set', { exact: true }).click({ button: 'right' }) await page.keyboard.press('Enter') - await page.locator('.ComponentEntry', { hasText: 'filter' }).click() + await selectComponentEntry(page, 'filter') await openDropdownInWidget(page, 'column') // Click with the assurance of component being in vision @@ -172,9 +173,7 @@ test('Exercise 2', async ({ page }) => { await expect(addComponent).toBeVisible() await addComponent.click() - const dataReadEntry = page.locator('.ComponentEntry', { hasText: /^Data\.read$/ }) - await expect(dataReadEntry).toBeVisible() - await dataReadEntry.click() + await selectComponentEntry(page, /^Data\.read$/) // Fill in the url await fillWidgetText(page, 'path‘‘', 'Samples/Data/sample_bank_data.xlsx') @@ -204,20 +203,20 @@ test('Exercise 2', async ({ page }) => { // Adding aggregate component await openComponentBrowser(page, 'readquery‘Sheet1’') - await page.locator('.ComponentEntry', { hasText: 'aggregate' }).click() + await selectComponentEntry(page, 'aggregate') // Choosing parameters const groupBy = page.getByText('group_by', { exact: true }) // Ensuring 'plus' is visible, to avoid clicking too early await expect(page.locator('div').getByLabel('Add a new item').last()).toBeVisible() - await groupBy.click() + await clickWithoutViewportConstraints(groupBy) const productBtn = page.getByRole('button', { name: 'product_name', exact: true }) - await productBtn.click() + await clickWithoutViewportConstraints(productBtn) // Close the dropdown - await page.getByText('aggregate').click() + await clickWithoutViewportConstraints(page.getByText('aggregate', { exact: true })) await addFirstElementToWidgetVector( page.locator('div.WidgetTopLevelArgument', { hasText: 'columns' }), @@ -229,7 +228,7 @@ test('Exercise 2', async ({ page }) => { // Adding sort component await openComponentBrowser(page, 'aggregate') - await page.locator('.ComponentEntry', { hasText: 'sort' }).click() + await selectComponentEntry(page, 'sort') await addFirstElementToWidgetVector( page.locator('div.WidgetTopLevelArgument', { hasText: 'columns' }), @@ -241,10 +240,12 @@ test('Exercise 2', async ({ page }) => { .filter({ hasText: /^‘product_name’$/ }) .nth(4) .click() - await page.getByRole('button', { name: 'Count', exact: true }).click() + await clickWithoutViewportConstraints(page.getByRole('button', { name: 'Count', exact: true })) - await page.getByText('direction', { exact: true }).click() - await page.getByRole('button', { name: '..Descending', exact: true }).click() + await clickWithoutViewportConstraints(page.getByText('direction', { exact: true })) + await clickWithoutViewportConstraints( + page.getByRole('button', { name: '..Descending', exact: true }), + ) await visualizeData(page) }) @@ -255,33 +256,36 @@ test('Exercise 2', async ({ page }) => { await page.mouse.wheel(0, -200) // Creating cross_tab component - const readComponent = page.getByText('read', { exact: true }).nth(2) - await readComponent.click({ button: 'right' }) - - await page.keyboard.press('Enter') - await page.locator('.ComponentEntry', { hasText: 'cross_tab' }).click() + await openComponentBrowserFromGraphNode(page, 'read', 2) + await selectComponentEntry(page, 'cross_tab') // Choosing the right parameters - const crossGroup = await page.getByText('group_by', { exact: true }).nth(1) + const crossGroup = page.getByText('group_by', { exact: true }).nth(1) // Ensuring 'plus' is visible, to avoid clicking too early await expect(page.locator('div').getByLabel('Add a new item').last()).toBeVisible() - await crossGroup.click() - await page.getByRole('button', { name: 'product_name', exact: true }).click() + await clickWithoutViewportConstraints(crossGroup) + await clickWithoutViewportConstraints( + page.getByRole('button', { name: 'product_name', exact: true }), + ) await openDropdownInWidget(page, 'names') const curRCode = page.getByRole('button', { name: 'currency_code' }).first() await expect(curRCode).toBeVisible() - await curRCode.click() + await clickWithoutViewportConstraints(curRCode) - await page.getByText('values', { exact: true }).click() - await page.getByRole('button', { name: '..Count_Distinct', exact: true }).click() + await clickWithoutViewportConstraints(page.getByText('values', { exact: true })) + await clickWithoutViewportConstraints( + page.getByRole('button', { name: '..Count_Distinct', exact: true }), + ) // Click the plus and select argument await addFirstElementToWidgetVector( page.locator('div.WidgetTopLevelArgument', { hasText: 'columns' }), ) - await page.getByRole('button', { name: 'account_id', exact: true }).click() + await clickWithoutViewportConstraints( + page.getByRole('button', { name: 'account_id', exact: true }), + ) await visualizeData(page) @@ -296,41 +300,45 @@ test('Exercise 2', async ({ page }) => { await page.mouse.wheel(0, -200) // Creating set component - const readComponent = page.getByText('read', { exact: true }).nth(2) - await readComponent.click({ button: 'right' }) - - await page.keyboard.press('Enter') - await page.locator('.ComponentEntry', { hasText: 'set' }).click() + await openComponentBrowserFromGraphNode(page, 'read', 2) + await selectComponentEntry(page, 'set') // Choosing right parameters await openDropdownInWidget(page, 'value') - await page.getByRole('button', { name: '', exact: true }).click() + await clickWithoutViewportConstraints( + page.getByRole('button', { name: '', exact: true }), + ) await openDropdownInWidget(page, 'input') - await page.getByRole('button', { name: 'currency_code', exact: true }).click() + await clickWithoutViewportConstraints( + page.getByRole('button', { name: 'currency_code', exact: true }), + ) await openDropdownInWidget(page, 'operation') - await page.getByRole('button', { name: 'if', exact: true }).click() + await clickWithoutViewportConstraints(page.getByRole('button', { name: 'if', exact: true })) await openDropdownInWidget(page, 'condition') - await page.getByRole('button', { name: '..Equal', exact: true }).click() + await clickWithoutViewportConstraints( + page.getByRole('button', { name: '..Equal', exact: true }), + ) await openDropdownInWidget(page, 'to') - await page.getByRole('button', { name: '' }).click() + await clickWithoutViewportConstraints(page.getByRole('button', { name: '' })) // Write in the textbox await fillWidgetText(page, '..Equal“”', 'G') - await page.getByText('true_value', { exact: true }).click() - await page.getByRole('button', { name: '', exact: true }).click() + await clickWithoutViewportConstraints(page.getByText('true_value', { exact: true })) + await clickWithoutViewportConstraints( + page.getByRole('button', { name: '', exact: true }), + ) // // Write in the textbox await fillWidgetText(page, '..If(..Equal“G”)“”', 'GBP', 1) - await page.getByText('false_value', { exact: true }).click() + await clickWithoutViewportConstraints(page.getByText('false_value', { exact: true })) const option = page.getByRole('button', { name: 'currency_code', exact: true }) - await option.hover() - await option.click() + await clickWithoutViewportConstraints(option) // Write in the textbox await fillWidgetText(page, 'as“”', 'currency_code') diff --git a/app/gui/tsconfig.app.json b/app/gui/tsconfig.app.json index 4f29aa5b0e82..df56b8c792b7 100644 --- a/app/gui/tsconfig.app.json +++ b/app/gui/tsconfig.app.json @@ -521,12 +521,12 @@ "./src/project-view/components/GraphEditor/GraphEdge/layout.ts", "./src/project-view/components/GraphEditor/GraphEdges.vue", "./src/project-view/components/GraphEditor/GraphNode.vue", - "./src/project-view/components/GraphEditor/GraphNodeAlignmentSubmenu.vue", "./src/project-view/components/GraphEditor/GraphNode/nodeMessage.ts", "./src/project-view/components/GraphEditor/GraphNode/nodeVisualization.ts", "./src/project-view/components/GraphEditor/GraphNodeComment.vue", "./src/project-view/components/GraphEditor/GraphNodeMessage.vue", "./src/project-view/components/GraphEditor/GraphNodeOutputPorts.vue", + "./src/project-view/components/GraphEditor/GraphNodeSubmenu.vue", "./src/project-view/components/GraphEditor/GraphNodes.vue", "./src/project-view/components/GraphEditor/GraphVisualization.vue", "./src/project-view/components/GraphEditor/GraphVisualization/VisualizationToolbar.vue", diff --git a/app/project-manager-shim/src/handler/jsonrpc.ts b/app/project-manager-shim/src/handler/jsonrpc.ts index e67cb79047cc..468813a8fb5f 100644 --- a/app/project-manager-shim/src/handler/jsonrpc.ts +++ b/app/project-manager-shim/src/handler/jsonrpc.ts @@ -3,11 +3,31 @@ export function toJSONRPCResult(result: unknown): string { return JSON.stringify({ jsonrpc: '2.0', id: 0, result }) } +function serializeErrorData(data: unknown): unknown { + if (data instanceof Error) { + return { + name: data.name, + message: data.message, + stack: data.stack, + ...(data.cause != null ? { cause: serializeErrorData(data.cause) } : {}), + } + } + if (Array.isArray(data)) { + return data.map(serializeErrorData) + } + if (data != null && typeof data === 'object') { + return Object.fromEntries( + Object.entries(data).map(([key, value]) => [key, serializeErrorData(value)]), + ) + } + return data +} + /** JSON-RPC error wrapper */ export function toJSONRPCError(message: string, data?: unknown): string { return JSON.stringify({ jsonrpc: '2.0', id: 0, - error: { code: 0, message, ...(data != null ? { data } : {}) }, + error: { code: 0, message, ...(data != null ? { data: serializeErrorData(data) } : {}) }, }) } diff --git a/app/project-manager-shim/src/projectService/ensoRunner.ts b/app/project-manager-shim/src/projectService/ensoRunner.ts index a653cd0eba9b..2e1da2ff5145 100644 --- a/app/project-manager-shim/src/projectService/ensoRunner.ts +++ b/app/project-manager-shim/src/projectService/ensoRunner.ts @@ -263,12 +263,25 @@ export class EnsoRunner implements Runner { return spawnCallback(cmd, cmdArgs) } + private processEnv(extraEnv?: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const env = { ...process.env, ...extraEnv } + const homeDir = os.homedir() + if (!env.HOME) { + env.HOME = homeDir + } + if (process.platform === 'win32' && !env.USERPROFILE) { + env.USERPROFILE = homeDir + } + return env + } + private async runCommand( args: readonly string[], options?: childProcess.SpawnOptionsWithoutStdio, ): Promise { + const env = this.processEnv(options?.env) const process = await this.runProcess(args, (cmd, cmdArgs) => - childProcess.spawn(cmd, cmdArgs, options), + childProcess.spawn(cmd, cmdArgs, { ...options, env }), ) return new Promise((resolve, reject) => { let stdout = '' @@ -306,7 +319,7 @@ export class EnsoRunner implements Runner { extraEnv?: readonly (readonly [string, string])[], ): Promise { const args = ['--run', projectPath] - const env = { ...process.env, ...(extraEnv ? Object.fromEntries(extraEnv) : {}) } + const env = this.processEnv(extraEnv ? Object.fromEntries(extraEnv) : undefined) const cwd = path.dirname(projectPath) const spawnedProcess = await this.runProcess(args, (cmd, cmdArgs) => childProcess.spawn(cmd, cmdArgs, { env, cwd, stdio: ['inherit', 'inherit', 'inherit'] }), @@ -371,11 +384,10 @@ export class EnsoRunner implements Runner { ...(extraArgs ?? []), ] - const env = { - ...process.env, + const env = this.processEnv({ LANGUAGE_SERVER_YDOC_PORT: ydocPort.toString(), ...(extraEnv ? Object.fromEntries(extraEnv) : {}), - } + }) const cwd = path.dirname(projectPath) const project = await OpenedProject.create( @@ -514,7 +526,7 @@ export class EnsoRunner implements Runner { const args = ['--version'] const cmd = this.ensoPath.endsWith('.bat') ? 'cmd.exe' : this.ensoPath const cmdArgs = this.ensoPath.endsWith('.bat') ? ['/c', this.ensoPath, ...args] : args - const process = childProcess.spawn(cmd, cmdArgs) + const process = childProcess.spawn(cmd, cmdArgs, { env: this.processEnv() }) let stdout = '' let stderr = '' diff --git a/app/project-manager-shim/src/projectService/index.ts b/app/project-manager-shim/src/projectService/index.ts index 81ed515d4df9..99d4d76d6737 100644 --- a/app/project-manager-shim/src/projectService/index.ts +++ b/app/project-manager-shim/src/projectService/index.ts @@ -6,6 +6,7 @@ import { PRODUCT_NAME } from 'enso-common/src/constants' import { toRfc3339 } from 'enso-common/src/utilities/data/dateTime' import * as crypto from 'node:crypto' +import { access } from 'node:fs/promises' import { EnsoRunner, findEnsoExecutable, @@ -130,7 +131,17 @@ export class ProjectService { } // Create project structure - await this.runner.createProject(projectPath, actualName, projectTemplate) + try { + await this.runner.createProject(projectPath, actualName, projectTemplate) + } catch (error) { + const projectsDirectoryExists = await access(projectsDirectory) + .then(() => true) + .catch(() => false) + throw new Error( + `Failed to create project '${actualName}' in '${projectsDirectory}' (exists=${projectsDirectoryExists}) at '${projectPath}'.`, + { cause: error }, + ) + } // Update metadata await repo.update(project) diff --git a/tools/enso4igv/.project b/tools/enso4igv/.project new file mode 100644 index 000000000000..25a9e9c2630b --- /dev/null +++ b/tools/enso4igv/.project @@ -0,0 +1,28 @@ + + + enso4igv + + + + + + org.eclipse.m2e.core.maven2Builder + + + + + + org.eclipse.m2e.core.maven2Nature + + + + 1774265912035 + + 30 + + org.eclipse.core.resources.regexFilterMatcher + node_modules|\.git|__CREATED_BY_JAVA_LANGUAGE_SERVER__ + + + + diff --git a/tools/enso4igv/.settings/org.eclipse.core.resources.prefs b/tools/enso4igv/.settings/org.eclipse.core.resources.prefs new file mode 100644 index 000000000000..99f26c0203a7 --- /dev/null +++ b/tools/enso4igv/.settings/org.eclipse.core.resources.prefs @@ -0,0 +1,2 @@ +eclipse.preferences.version=1 +encoding/=UTF-8 diff --git a/tools/enso4igv/.settings/org.eclipse.m2e.core.prefs b/tools/enso4igv/.settings/org.eclipse.m2e.core.prefs new file mode 100644 index 000000000000..f897a7f1cb23 --- /dev/null +++ b/tools/enso4igv/.settings/org.eclipse.m2e.core.prefs @@ -0,0 +1,4 @@ +activeProfiles= +eclipse.preferences.version=1 +resolveWorkspaceProjects=true +version=1 From 6ddec3226498cde8c0eec7167ab34e14be922d65 Mon Sep 17 00:00:00 2001 From: Ilya Bogdanov Date: Tue, 7 Apr 2026 17:19:14 +0400 Subject: [PATCH 11/11] cleanups --- .../src/handler/jsonrpc.ts | 22 +------------------ 1 file changed, 1 insertion(+), 21 deletions(-) diff --git a/app/project-manager-shim/src/handler/jsonrpc.ts b/app/project-manager-shim/src/handler/jsonrpc.ts index 468813a8fb5f..e67cb79047cc 100644 --- a/app/project-manager-shim/src/handler/jsonrpc.ts +++ b/app/project-manager-shim/src/handler/jsonrpc.ts @@ -3,31 +3,11 @@ export function toJSONRPCResult(result: unknown): string { return JSON.stringify({ jsonrpc: '2.0', id: 0, result }) } -function serializeErrorData(data: unknown): unknown { - if (data instanceof Error) { - return { - name: data.name, - message: data.message, - stack: data.stack, - ...(data.cause != null ? { cause: serializeErrorData(data.cause) } : {}), - } - } - if (Array.isArray(data)) { - return data.map(serializeErrorData) - } - if (data != null && typeof data === 'object') { - return Object.fromEntries( - Object.entries(data).map(([key, value]) => [key, serializeErrorData(value)]), - ) - } - return data -} - /** JSON-RPC error wrapper */ export function toJSONRPCError(message: string, data?: unknown): string { return JSON.stringify({ jsonrpc: '2.0', id: 0, - error: { code: 0, message, ...(data != null ? { data: serializeErrorData(data) } : {}) }, + error: { code: 0, message, ...(data != null ? { data } : {}) }, }) }