diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index ee6adbc7..ca02e6ef 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -15,8 +15,12 @@ concurrency: jobs: e2e: - name: 'Playwright (Linux)' - runs-on: ubuntu-latest + name: 'Playwright (${{ matrix.os }})' + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest] + runs-on: ${{ matrix.os }} # Bound the job so a stuck Electron launch/close fails fast instead of # tying up a runner; this suite specifically guards shutdown hangs. timeout-minutes: 15 @@ -33,26 +37,60 @@ jobs: node-version: '24.x' cache: 'yarn' - run: yarn install --frozen-lockfile - # electron's postinstall (extract-zip) leaves dist without the `electron` - # binary on this runner even though the downloaded zip is complete and - # valid, so launch fails with "Electron failed to install correctly". Let - # install.js fetch the zip into the cache, then extract it with the system - # `unzip` and point path.txt at the binary. + # The electron postinstall (extract-zip) intermittently leaves dist without + # the binary on CI runners (it happens on macOS too, not just Linux), so + # launch fails with "Electron failed to install correctly". Run install.js + # explicitly on every OS to fetch and extract the binary. - name: Install the Electron binary + run: node node_modules/electron/install.js + # On Linux, if extract-zip still dropped the binary, fall back to system + # unzip (the zip is the linux-x64 build, binary at dist/electron). + - name: Repair the Electron binary on Linux if needed + if: matrix.os == 'ubuntu-latest' run: | set -euo pipefail - node node_modules/electron/install.js || true - zip=$(ls ~/.cache/electron/*/electron-*-linux-x64.zip | head -1) + if [ ! -x node_modules/electron/dist/electron ]; then + echo "linux electron binary missing, repairing from the cached zip" + zip=$(ls ~/.cache/electron/*/electron-*-linux-x64.zip | head -1) + rm -rf node_modules/electron/dist + mkdir -p node_modules/electron/dist + unzip -q -o "$zip" -d node_modules/electron/dist + printf 'electron' > node_modules/electron/path.txt + else + echo "linux electron binary present, no repair needed" + fi + test -x node_modules/electron/dist/electron + # extract-zip produces a broken Electron.app on the macOS runner: the + # launcher at Contents/MacOS/Electron is present but Contents/Frameworks/ + # Electron Framework.framework is incomplete, so dyld fails with "Library + # not loaded" at launch, and path.txt is never written either. Re-extract + # the cached darwin zip with ditto, which reproduces a complete .app + # (framework binary plus the Versions/Current symlinks plain unzip mangles) + # locally, then write path.txt (its macOS value is constant) and verify + # both the launcher and the framework binary are in place. + - name: Repair the Electron install on macOS + if: matrix.os == 'macos-latest' + run: | + set -euo pipefail + # Ask @electron/get (electron's own download dep) for the cached zip, + # fetching it if absent. It stores the zip under a content-hash subdir, + # so globbing the cache root misses it; let the library resolve the path. + zip=$(node -e "const {downloadArtifact}=require('@electron/get');const {version}=require('electron/package.json');downloadArtifact({version,artifactName:'electron',platform:'darwin',arch:process.arch}).then(p=>process.stdout.write(p)).catch(e=>{console.error(e);process.exit(1)})") + echo "re-extracting electron from $zip" rm -rf node_modules/electron/dist mkdir -p node_modules/electron/dist - unzip -q -o "$zip" -d node_modules/electron/dist - printf 'electron' > node_modules/electron/path.txt - test -x node_modules/electron/dist/electron + ditto -x -k "$zip" node_modules/electron/dist + printf 'Electron.app/Contents/MacOS/Electron' > node_modules/electron/path.txt + test -x "node_modules/electron/dist/Electron.app/Contents/MacOS/Electron" + test -f "node_modules/electron/dist/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework" + node -e "console.log('resolved electron:', require('electron'))" - run: yarn build # Electron needs system libraries; xvfb provides a display on the headless # runner. The app launches up to four windows, so a real display beats - # --headless here. - - run: npx playwright install-deps + # --headless here. Both are Linux-only: macOS runners have a display and do + # not use this apt-based dependency installer. + - if: matrix.os == 'ubuntu-latest' + run: npx playwright install-deps # A minimal Python env with jupyterlab so the env-backed tests run (a venv # + pip is lighter than a full conda env for a smoke). The seeded # app-data.json points the app at this interpreter. @@ -67,11 +105,19 @@ jobs: python3 -m venv "$RUNNER_TEMP/jlab-venv" "$RUNNER_TEMP/jlab-venv/bin/pip" install --quiet "jupyterlab==$ver" echo "JLAB_TEST_PYTHON_PATH=$RUNNER_TEMP/jlab-venv/bin/python" >> "$GITHUB_ENV" - - run: xvfb-run -a yarn test:e2e + # xvfb is Linux-only; on macOS the runner has a real display, so run the + # suite directly. Same yarn test:e2e command on both legs. + - name: Run the e2e suite + run: | + if [ "$RUNNER_OS" = "Linux" ]; then + xvfb-run -a yarn test:e2e + else + yarn test:e2e + fi - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4 if: ${{ !cancelled() }} with: - name: playwright-report + name: playwright-report-${{ matrix.os }} path: | playwright-report/ test-results/ diff --git a/test/e2e/helpers.ts b/test/e2e/helpers.ts index 87bd40a7..5d2ef8dc 100644 --- a/test/e2e/helpers.ts +++ b/test/e2e/helpers.ts @@ -4,6 +4,16 @@ import { mkdtempSync, rmSync, writeFileSync } from 'fs'; import { join } from 'path'; import { tmpdir } from 'os'; +// A booted JupyterLab server URL: the labview navigates here once a session is +// up. Shared so every server-backed test waits on the same pattern. +export const LAB_URL = /https?:\/\/(127\.0\.0\.1|localhost):\d+/; + +// The env-backed tests need a real Python with jupyterlab; CI provisions one and +// points JLAB_TEST_PYTHON_PATH at it. Shared so the skip reason reads the same +// everywhere. +export const NEEDS_PYTHON = + 'set JLAB_TEST_PYTHON_PATH to a python with jupyterlab'; + // A fresh userData dir per launch keeps tests independent. Electron's native // --user-data-dir flag is honored because the app reads app.getPath('userData') // (see getUserDataDir in utils) and only overrides it on Snap. Returns the app @@ -17,10 +27,33 @@ import { tmpdir } from 'os'; // actions; `userSetPythonEnvs` registers the env so the session registry // resolves it) and `settings.json` (`pythonPath` is what a new session reads // via workspace settings to boot its server). +// +// The spawned Jupyter server inherits the launch env via `{ ...process.env }` +// in src/main/server.ts, so redirecting the user-layer Jupyter dirs plus HOME +// here isolates the server's runtime/config writes to a per-launch temp instead +// of the runner's real ~/.jupyter and ~/Library/Jupyter. DATA and RUNTIME pass +// straight through. CONFIG is special: server.ts hardcodes the server's +// JUPYTER_CONFIG_DIR to `process.env.JLAB_DESKTOP_CONFIG_DIR || getUserDataDir()`, +// so a plain JUPYTER_CONFIG_DIR here would be ignored for that server; we set +// JLAB_DESKTOP_CONFIG_DIR (the app's own override) to point it at the temp, and +// keep JUPYTER_CONFIG_DIR for any child process that reads it directly. This +// mirrors pytest-jupyter's `jp_environ` fixture. We do +// NOT set JUPYTER_PATH / JUPYTER_CONFIG_PATH: those control the search path for +// installed extensions, and the venv ships its jupyterlab assets under the env +// prefix; overriding them would hide the lab front end. On macOS, setting HOME +// does not move Electron's own app.getPath('home') (it uses NSHomeDirectory), +// but HOME does reach the python server through `...process.env`, which is what +// we want isolated. The jupyter temp dir is returned so the caller can remove +// it alongside userDataDir. export async function launchApp(opts?: { pythonPath?: string; -}): Promise<{ app: ElectronApplication; userDataDir: string }> { +}): Promise<{ + app: ElectronApplication; + userDataDir: string; + jupyterDir: string; +}> { const userDataDir = mkdtempSync(join(tmpdir(), 'jlab-e2e-')); + const jupyterDir = mkdtempSync(join(tmpdir(), 'jlab-e2e-jupyter-')); if (opts?.pythonPath) { writeFileSync( join(userDataDir, 'app-data.json'), @@ -43,18 +76,29 @@ export async function launchApp(opts?: { } try { const app = await electron.launch({ - args: ['.', `--user-data-dir=${userDataDir}`] + args: ['.', `--user-data-dir=${userDataDir}`], + env: { + ...process.env, + HOME: jupyterDir, + JLAB_DESKTOP_CONFIG_DIR: jupyterDir, + JUPYTER_CONFIG_DIR: jupyterDir, + JUPYTER_DATA_DIR: jupyterDir, + JUPYTER_RUNTIME_DIR: join(jupyterDir, 'runtime') + } }); await stubAllDialogs(app); - return { app, userDataDir }; + return { app, userDataDir, jupyterDir }; } catch (error) { - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); throw error; } } -export function cleanup(userDataDir: string): void { +export function cleanup(userDataDir: string, jupyterDir?: string): void { rmSync(userDataDir, { recursive: true, force: true }); + if (jupyterDir) { + rmSync(jupyterDir, { recursive: true, force: true }); + } } // The app opens several windows (titlebar, welcome, session, manager) and most @@ -81,3 +125,29 @@ export async function pageByTitle( } throw new Error(`no window title matched ${pattern}`); } + +// Wait for any open page whose URL matches `pattern`. Unlike +// electron-playwright-helpers' waitForWindowByUrl, which only resolves on a new +// "window" event, this also catches an in-place navigation of an existing page +// (e.g. the session content view swapping to a remote server URL), so it is the +// reliable wait for "a view reached this URL" regardless of how it got there. +export async function pageByUrl( + app: ElectronApplication, + pattern: RegExp, + timeout = 90000 +) { + const deadline = Date.now() + timeout; + while (Date.now() < deadline) { + for (const page of app.windows()) { + try { + if (pattern.test(page.url())) { + return page; + } + } catch { + // page may be closing; retry on the next pass + } + } + await new Promise(resolve => setTimeout(resolve, 150)); + } + throw new Error(`no window URL matched ${pattern}`); +} diff --git a/test/e2e/python-env.test.ts b/test/e2e/python-env.test.ts index 0ca46b61..4ec9efe7 100644 --- a/test/e2e/python-env.test.ts +++ b/test/e2e/python-env.test.ts @@ -1,6 +1,12 @@ import { expect, test } from '@playwright/test'; -import { waitForWindowByUrl } from 'electron-playwright-helpers'; -import { cleanup, launchApp, pageByTitle } from './helpers'; +import { + cleanup, + LAB_URL, + launchApp, + NEEDS_PYTHON, + pageByTitle, + pageByUrl +} from './helpers'; // Needs a real Python env with jupyterlab. CI provisions one and points // JLAB_TEST_PYTHON_PATH at it; locally, set it to any python that has jupyterlab @@ -8,12 +14,9 @@ import { cleanup, launchApp, pageByTitle } from './helpers'; const pythonPath = process.env.JLAB_TEST_PYTHON_PATH; test('with a seeded Python env, New notebook opens the labview', async () => { - test.skip( - !pythonPath, - 'set JLAB_TEST_PYTHON_PATH to a python with jupyterlab' - ); + test.skip(!pythonPath, NEEDS_PYTHON); test.setTimeout(120000); - const { app, userDataDir } = await launchApp({ pythonPath }); + const { app, userDataDir, jupyterDir } = await launchApp({ pythonPath }); try { const welcome = await pageByTitle(app, /welcome/i); // The app validated the seeded env and emitted EnableLocalServerActions, @@ -24,14 +27,10 @@ test('with a seeded Python env, New notebook opens the labview', async () => { // The session boots a real Jupyter server (via the seeded env) and the // labview WebContentsView navigates to it. Assert it reached the local // server URL rather than driving the JupyterLab DOM through Electron. - const lab = await waitForWindowByUrl( - app, - /https?:\/\/(127\.0\.0\.1|localhost):\d+/, - { timeout: 90000 } - ); - expect(lab.url()).toMatch(/:\d+/); + const lab = await pageByUrl(app, LAB_URL); + expect(lab.url()).toMatch(/\/lab/); } finally { await app.close(); - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); } }); diff --git a/test/e2e/smoke.test.ts b/test/e2e/smoke.test.ts index 0cb12642..5c6cda24 100644 --- a/test/e2e/smoke.test.ts +++ b/test/e2e/smoke.test.ts @@ -2,18 +2,18 @@ import { expect, test } from '@playwright/test'; import { cleanup, launchApp, pageByTitle } from './helpers'; test('app launches and opens at least one window', async () => { - const { app, userDataDir } = await launchApp(); + const { app, userDataDir, jupyterDir } = await launchApp(); try { await app.firstWindow(); expect(app.windows().length).toBeGreaterThan(0); } finally { await app.close(); - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); } }); test('on first run the Welcome window renders', async () => { - const { app, userDataDir } = await launchApp(); + const { app, userDataDir, jupyterDir } = await launchApp(); try { // Selecting by title rather than firstWindow(): the app opens several // windows and only this one is the welcome view. @@ -24,12 +24,12 @@ test('on first run the Welcome window renders', async () => { await expect(welcome.locator('#new-notebook-link')).toBeVisible(); } finally { await app.close(); - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); } }); test('the session window composes multiple views (titlebar + welcome + content)', async () => { - const { app, userDataDir } = await launchApp(); + const { app, userDataDir, jupyterDir } = await launchApp(); try { // Once the welcome view has settled, the BrowserView/WebContentsView // composition should expose several views (titlebar, welcome, content area) @@ -41,19 +41,19 @@ test('the session window composes multiple views (titlebar + welcome + content)' .toBeGreaterThanOrEqual(3); } finally { await app.close(); - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); } }); test('app shuts down cleanly without hanging', async () => { - const { app, userDataDir } = await launchApp(); + const { app, userDataDir, jupyterDir } = await launchApp(); try { await app.firstWindow(); } finally { // Close in finally so the Electron process is always terminated, even if // an earlier step throws. A hang here surfaces via the job timeout. await app.close(); - cleanup(userDataDir); + cleanup(userDataDir, jupyterDir); } // close() has resolved, so the process exited and the windows are gone. expect(app.windows().length).toBe(0); diff --git a/test/unit/server-env.test.ts b/test/unit/server-env.test.ts new file mode 100644 index 00000000..ac9995c1 --- /dev/null +++ b/test/unit/server-env.test.ts @@ -0,0 +1,177 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { execFile } from 'child_process'; + +// Narrow seam under test: JupyterServer.start() must hand the spawned process an +// env that redirects JUPYTER_CONFIG_DIR to the app's userData dir (or the +// JLAB_DESKTOP_CONFIG_DIR override), so the embedded server keeps its config out +// of the runner's real ~/.jupyter. The full start() path is heavily coupled +// (free port, launch-script file, settings, conda dialogs), so we mock those +// boundaries and assert only on the env passed to execFile. + +const USER_DATA_DIR = '/tmp/fake-user-data'; + +vi.mock('child_process', async () => { + const actual = await vi.importActual( + 'child_process' + ); + return { ...actual, execFile: vi.fn() }; +}); + +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + return { + ...actual, + existsSync: vi.fn(() => true), + chmodSync: vi.fn(), + unlinkSync: vi.fn() + }; +}); + +vi.mock('../../src/main/utils', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + getFreePort: vi.fn(async () => 8899), + getUserDataDir: vi.fn(() => '/tmp/fake-user-data'), + createTempFile: vi.fn(() => '/tmp/launch.sh'), + getEnvironmentPath: vi.fn(() => '/envs/e2e'), + activatePathForEnvPath: vi.fn(() => '/envs/e2e/bin/activate'), + waitForDuration: vi.fn(() => new Promise(() => undefined)) + }; +}); + +vi.mock('../../src/main/config/settings', () => ({ + serverLaunchArgsDefault: [], + serverLaunchArgsFixed: [], + SettingType: { + serverArgs: 'serverArgs', + overrideDefaultServerArgs: 'overrideDefaultServerArgs', + serverEnvVars: 'serverEnvVars' + }, + userSettings: { + resolvedWorkingDirectory: '/work' + }, + WorkspaceSettings: vi.fn().mockImplementation(function () { + return { + getValue: (setting: string) => { + if (setting === 'serverArgs') { + return ''; + } + if (setting === 'overrideDefaultServerArgs') { + return false; + } + if (setting === 'serverEnvVars') { + return {}; + } + return undefined; + } + }; + }) +})); + +vi.mock('../../src/main/env', () => ({ + condaEnvPathForCondaExePath: vi.fn(() => ''), + getCondaPath: vi.fn(() => '') +})); + +vi.mock('../../src/main/config/appdata', () => ({ + appData: { + condaPath: null, + systemPythonPath: null, + pythonPath: null + } +})); + +vi.mock('electron', () => ({ + app: { + getPath: vi.fn(() => '/tmp/fake-user-data'), + getAppPath: vi.fn(() => '/app') + }, + dialog: { showMessageBox: vi.fn(), showMessageBoxSync: vi.fn() }, + ipcMain: { emit: vi.fn(), on: vi.fn(), handle: vi.fn() } +})); + +vi.mock('../../src/main/pythonenvdialog/pythonenvdialog', () => ({ + ManagePythonEnvironmentDialog: { Tab: { Settings: 'settings' } } +})); + +import { JupyterServer } from '../../src/main/server'; +import { IEnvironmentType } from '../../src/main/tokens'; + +const mockExecFile = vi.mocked(execFile); + +function makeChild() { + return { + on: vi.fn(), + stdout: { on: vi.fn() }, + stderr: { on: vi.fn() } + } as any; +} + +// Lets the async start() executor run far enough to reach the execFile call: +// it awaits getFreePort (one mocked promise) before building the env. +async function flush() { + for (let i = 0; i < 5; i++) { + await Promise.resolve(); + } +} + +beforeEach(() => { + vi.clearAllMocks(); + mockExecFile.mockReturnValue(makeChild()); +}); + +afterEach(() => { + vi.restoreAllMocks(); +}); + +describe('JupyterServer.start env isolation', () => { + it('sets JUPYTER_CONFIG_DIR on the spawned server env to the userData dir', async () => { + const server = new JupyterServer({ + environment: { + path: '/envs/e2e/bin/python', + name: 'e2e', + type: IEnvironmentType.VirtualEnv, + versions: {}, + defaultKernel: 'python3' + } + }); + + server.start(); + await flush(); + + expect(mockExecFile).toHaveBeenCalledTimes(1); + const options = mockExecFile.mock.calls[0][1] as { env: NodeJS.ProcessEnv }; + expect(options.env.JUPYTER_CONFIG_DIR).toBe(USER_DATA_DIR); + }); + + it('prefers the JLAB_DESKTOP_CONFIG_DIR override over the userData dir', async () => { + const original = process.env.JLAB_DESKTOP_CONFIG_DIR; + process.env.JLAB_DESKTOP_CONFIG_DIR = '/override/config'; + try { + const server = new JupyterServer({ + environment: { + path: '/envs/e2e/bin/python', + name: 'e2e', + type: IEnvironmentType.VirtualEnv, + versions: {}, + defaultKernel: 'python3' + } + }); + + server.start(); + await flush(); + + const options = mockExecFile.mock.calls[0][1] as { + env: NodeJS.ProcessEnv; + }; + expect(options.env.JUPYTER_CONFIG_DIR).toBe('/override/config'); + } finally { + if (original === undefined) { + delete process.env.JLAB_DESKTOP_CONFIG_DIR; + } else { + process.env.JLAB_DESKTOP_CONFIG_DIR = original; + } + } + }); +});