Skip to content

Commit 9549bc1

Browse files
notluquisclaudekrassowski
authored
feat: add test infrastructure (Vitest + Playwright) (#961)
* feat: add test infrastructure (Vitest + Playwright) Lands the test infrastructure from `revival/electron-41` onto master so follow-up Phase 2 PRs (test expansion, CI matrix hardening) have something to build on. What this brings: - Vitest 4 for unit tests, with an electron mock in `test/setup/`. - 21 unit tests for `src/main/env.ts` (`validateNewPythonEnvironmentName`, `validatePythonEnvironmentInstallDirectory`, `environmentSatisfiesRequirements`, `validateCondaChannels`). - Playwright + electron-playwright-helpers for an E2E smoke suite (one initial test: app launches and shows at least one window). - A `test` job in `publish.yml` that runs `yarn test:unit` before the release-build matrix. - Phase 1 dependency security pins via yarn 1 `resolutions`: `serialize-javascript ^6.0.2`, `micromatch ^4.0.8`. (Both are build/dev-time only on master; this is for completeness, fast-xml-parser 4 -> 5 already landed via #849.) What this is NOT: - Not the full 341-test suite (lands in a follow-up PR). - Not the full E2E smoke (window detection, exit codes, IPC integrity follow in a later PR). - Not the cross-platform CI matrix (separate PR). - Not Phase 2b security advisory work (handled via private advisory forks, not public PRs). Verification: - `yarn install` clean. - `yarn test:unit`: 21/21 pass. - `yarn build`: webpack 5.76.0 compiles all 12 preloads green. - `npx tsc --noEmit`: exits 0. Note: AI-assisted (Claude Code; original commit on revival co-authored with Claude Sonnet 4.6). The PR series this is part of will split out the remaining test files, husky/lint-staged, and the matrix changes into reviewable atomic chunks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * ci: bump unit test job actions to v5 GitHub Actions began deprecating the Node.js 20 runtime on 2025-09-19. The unit test job introduced in this branch uses `actions/checkout@v4` and `actions/setup-node@v4`, both of which run on Node.js 20 internally and trigger a deprecation warning on ubuntu-latest runners. v5 of both actions ships with Node.js 24 support. This bump is scoped to the new `test` job only. The release `publish` job still uses `actions/checkout@v4`; that one has its own open Dependabot PR (#926) and is out of scope here. Note: AI-assisted (Claude Code). Manually verified `yarn test:unit` locally still reports 21/21 pass after the bump. * fix(test): use repo-relative 2-dot paths in env.test.ts The env unit test imported the SUT and mocked its config modules with 3-dot paths (`../../../src/main/...`) from `test/unit/env.test.ts`. By plain Node path resolution that resolves above the repo root. Vitest happened to find the modules anyway through its own resolver fallback, but the path is incorrect on its face and was flagged in PR review. Replaces with 2-dot paths (`../../src/main/...`), which match the actual file system layout from `test/unit/`. Verified: - `yarn test:unit`: 21/21 pass with the new paths. - Mutation check (rename `validateNewPythonEnvironmentName` in the SUT): test suite goes red, confirming the imports still resolve to the real `src/main/env.ts` after this change. Note: AI-assisted (Claude Code). * fix(test): drop redundant electron alias from vitest config The vitest config aliased the bare `electron` specifier to `test/setup/electron-mock.ts`. That setup file does not export `app`, `BrowserWindow`, etc.; it only calls `vi.mock('electron', ...)` to register a mock factory through `setupFiles`. With the alias in place, any direct `import { app } from 'electron'` would have resolved to an empty module instead of the mock object, masking real bugs. The `setupFiles: ['test/setup/electron-mock.ts']` entry already runs the `vi.mock('electron', ...)` registration before tests, so removing the alias gives the mock factory back its job and avoids the empty-module trap. Verified `yarn test:unit` still 21/21 with the alias removed. Note: AI-assisted (Claude Code). Issue surfaced by Copilot review on PR-C1. * Update .github/workflows/publish.yml Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: Michał Krassowski <5832902+krassowski@users.noreply.github.com>
1 parent c6e641a commit 9549bc1

8 files changed

Lines changed: 1019 additions & 13 deletions

File tree

.github/workflows/publish.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,22 @@ on:
1010
workflow_dispatch:
1111

1212
jobs:
13+
test:
14+
name: 'Unit tests'
15+
runs-on: ubuntu-latest
16+
permissions:
17+
contents: read
18+
steps:
19+
- uses: actions/checkout@v5
20+
- uses: actions/setup-node@v5
21+
with:
22+
node-version: '20.x'
23+
cache: 'yarn'
24+
- run: yarn install --frozen-lockfile
25+
- run: yarn test:unit
26+
1327
publish:
28+
needs: test
1429
strategy:
1530
matrix:
1631
cfg:

package.json

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,10 @@
55
"main": "./build/out/main/main.js",
66
"scripts": {
77
"start": "electron .",
8-
"test": "echo \"Error: no test specified\" && exit 1",
8+
"test": "yarn test:unit",
9+
"test:unit": "vitest run",
10+
"test:unit:watch": "vitest",
11+
"test:e2e": "playwright test",
912
"clean": "rimraf build dist",
1013
"watch:tsc": "tsc -w",
1114
"watch:assets": "node ./scripts/extract.js && node ./scripts/copyassets.js watch",
@@ -105,7 +108,7 @@
105108
"base": "core22",
106109
"environment": {
107110
"SHELL": "/bin/bash",
108-
"GTK_USE_PORTAL":"1"
111+
"GTK_USE_PORTAL": "1"
109112
},
110113
"hooks": "build/snap-hooks"
111114
},
@@ -180,6 +183,8 @@
180183
"license": "BSD-3-Clause",
181184
"devDependencies": {
182185
"@jupyter-notebook/web-components": "0.9.1",
186+
"@leeoniya/ufuzzy": "1.0.14",
187+
"@playwright/test": "^1.59.1",
183188
"@types/ejs": "^3.1.0",
184189
"@types/js-yaml": "^4.0.3",
185190
"@types/node": "^14.14.31",
@@ -190,10 +195,11 @@
190195
"@types/yargs": "^17.0.18",
191196
"@typescript-eslint/eslint-plugin": "~5.28.0",
192197
"@typescript-eslint/parser": "~5.28.0",
193-
"@leeoniya/ufuzzy": "1.0.14",
198+
"@vitest/coverage-v8": "^4.1.5",
194199
"electron": "^27.0.2",
195200
"electron-builder": "^24.9.1",
196201
"electron-notarize": "^1.2.2",
202+
"electron-playwright-helpers": "^2.1.0",
197203
"eslint": "~8.17.0",
198204
"eslint-config-prettier": "~8.5.0",
199205
"eslint-plugin-prettier": "~4.0.0",
@@ -207,9 +213,14 @@
207213
"rimraf": "~3.0.0",
208214
"shx": "^0.3.4",
209215
"typescript": "~4.2.2",
216+
"vitest": "^4.1.5",
210217
"webpack": "^5.76.0",
211218
"webpack-cli": "^4.5.0"
212219
},
220+
"resolutions": {
221+
"serialize-javascript": "^6.0.2",
222+
"micromatch": "^4.0.8"
223+
},
213224
"dependencies": {
214225
"@lumino/signaling": "^1.10.0",
215226
"ejs": "^3.1.10",

playwright.config.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { defineConfig } from '@playwright/test';
2+
3+
export default defineConfig({
4+
testDir: './test/e2e',
5+
timeout: 60000,
6+
retries: process.env.CI ? 2 : 0,
7+
reporter: process.env.CI ? 'github' : 'list',
8+
use: {
9+
trace: 'on-first-retry',
10+
video: 'on-first-retry'
11+
},
12+
projects: [
13+
{
14+
name: 'electron',
15+
testMatch: '**/*.test.ts'
16+
}
17+
],
18+
outputDir: 'test-results/'
19+
});

test/e2e/smoke.test.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { _electron as electron, expect, test } from '@playwright/test';
2+
import { stubAllDialogs } from 'electron-playwright-helpers';
3+
import { mkdtempSync, rmSync } from 'fs';
4+
import { join } from 'path';
5+
import { tmpdir } from 'os';
6+
7+
let tempUserData: string;
8+
9+
test.beforeEach(() => {
10+
tempUserData = mkdtempSync(join(tmpdir(), 'jlab-e2e-'));
11+
});
12+
13+
test.afterEach(async () => {
14+
rmSync(tempUserData, { recursive: true, force: true });
15+
});
16+
17+
test('app launches and shows a window', async () => {
18+
const app = await electron.launch({
19+
args: ['.'],
20+
env: {
21+
...process.env,
22+
JLAB_DESKTOP_HOME: tempUserData,
23+
ELECTRON_IS_TEST: '1'
24+
}
25+
});
26+
27+
await stubAllDialogs(app);
28+
const window = await app.firstWindow();
29+
await window.waitForLoadState('domcontentloaded');
30+
31+
expect(app.windows().length).toBeGreaterThan(0);
32+
await app.close();
33+
});
34+
35+
test('app exits with code 0', async () => {
36+
const app = await electron.launch({
37+
args: ['.'],
38+
env: {
39+
...process.env,
40+
JLAB_DESKTOP_HOME: tempUserData,
41+
ELECTRON_IS_TEST: '1'
42+
}
43+
});
44+
45+
await stubAllDialogs(app);
46+
await app.firstWindow();
47+
48+
const exitCode = await app.close();
49+
expect(exitCode).toBe(0);
50+
});
51+
52+
test('first-run shows env selection when no env configured', async () => {
53+
const app = await electron.launch({
54+
args: ['.'],
55+
env: {
56+
...process.env,
57+
JLAB_DESKTOP_HOME: tempUserData,
58+
ELECTRON_IS_TEST: '1',
59+
// empty userData = no prior config = first run
60+
APPDATA: tempUserData
61+
}
62+
});
63+
64+
await stubAllDialogs(app);
65+
const window = await app.firstWindow();
66+
67+
// welcome view or env select should appear within 15s
68+
await expect(
69+
window.locator(
70+
'#welcome-view, #env-select-dialog, [data-testid="env-select"]'
71+
)
72+
).toBeVisible({ timeout: 15000 });
73+
74+
await app.close();
75+
});

test/setup/electron-mock.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { vi } from 'vitest';
2+
import { tmpdir } from 'os';
3+
import { join } from 'path';
4+
5+
const userDataPath = join(tmpdir(), 'jlab-test-userdata');
6+
const logFilePath = join(tmpdir(), 'jlab-test.log');
7+
8+
vi.mock('electron', () => ({
9+
app: {
10+
getPath: vi.fn((name: string) => join(userDataPath, name)),
11+
getVersion: vi.fn(() => '4.4.7'),
12+
getName: vi.fn(() => 'JupyterLab'),
13+
isPackaged: false,
14+
whenReady: vi.fn(() => Promise.resolve())
15+
},
16+
ipcMain: {
17+
on: vi.fn(),
18+
handle: vi.fn(),
19+
removeHandler: vi.fn(),
20+
removeAllListeners: vi.fn(),
21+
emit: vi.fn()
22+
},
23+
dialog: {
24+
showOpenDialog: vi.fn(),
25+
showMessageBox: vi.fn(),
26+
showMessageBoxSync: vi.fn(() => 0)
27+
},
28+
BrowserWindow: vi.fn().mockImplementation(() => ({
29+
loadURL: vi.fn(),
30+
webContents: { send: vi.fn(), on: vi.fn() },
31+
on: vi.fn(),
32+
once: vi.fn(),
33+
show: vi.fn(),
34+
close: vi.fn(),
35+
isDestroyed: vi.fn(() => false)
36+
})),
37+
shell: { openExternal: vi.fn(), openPath: vi.fn() },
38+
nativeTheme: { shouldUseDarkColors: false },
39+
screen: {
40+
getPrimaryDisplay: vi.fn(() => ({
41+
workAreaSize: { width: 1920, height: 1080 }
42+
}))
43+
}
44+
}));
45+
46+
vi.mock('electron-log', () => ({
47+
default: {
48+
info: vi.fn(),
49+
error: vi.fn(),
50+
debug: vi.fn(),
51+
warn: vi.fn(),
52+
transports: {
53+
file: {
54+
level: 'info',
55+
getFile: () => ({ path: logFilePath })
56+
},
57+
console: { level: false }
58+
}
59+
}
60+
}));

test/unit/env.test.ts

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
import { beforeEach, describe, expect, it, vi } from 'vitest';
2+
import * as fs from 'fs';
3+
4+
vi.mock('fs', async () => {
5+
const actual = await vi.importActual<typeof import('fs')>('fs');
6+
return {
7+
...actual,
8+
existsSync: vi.fn(),
9+
lstatSync: vi.fn()
10+
};
11+
});
12+
vi.mock('../../src/main/config/settings', () => ({
13+
userSettings: {
14+
getValue: vi.fn(() => null),
15+
setValue: vi.fn()
16+
},
17+
SettingType: {
18+
condaPath: 'condaPath',
19+
pythonPath: 'pythonPath',
20+
envType: 'envType'
21+
}
22+
}));
23+
vi.mock('../../src/main/config/appdata', () => ({
24+
appData: {
25+
discoveredPythonPaths: [],
26+
condaPath: null
27+
}
28+
}));
29+
30+
import {
31+
environmentSatisfiesRequirements,
32+
validateCondaChannels,
33+
validateNewPythonEnvironmentName,
34+
validatePythonEnvironmentInstallDirectory
35+
} from '../../src/main/env';
36+
37+
const mockFs = vi.mocked(fs);
38+
39+
describe('validateNewPythonEnvironmentName', () => {
40+
beforeEach(() => {
41+
mockFs.existsSync = vi.fn(() => false);
42+
});
43+
44+
it.each([
45+
['valid-name', true],
46+
['valid_name_123', true],
47+
['MyEnv', true],
48+
['', false],
49+
[' ', false],
50+
['name with spaces', false],
51+
['name/slash', false],
52+
['name.dot', false],
53+
['name@at', false]
54+
])('"%s" → valid: %s', (name, expected) => {
55+
expect(validateNewPythonEnvironmentName(name).valid).toBe(expected);
56+
});
57+
58+
it('returns invalid when directory already exists', () => {
59+
mockFs.existsSync = vi.fn(() => true);
60+
const result = validateNewPythonEnvironmentName('existing-env');
61+
expect(result.valid).toBe(false);
62+
expect(result.message).toMatch(/already exists/i);
63+
});
64+
});
65+
66+
describe('validatePythonEnvironmentInstallDirectory', () => {
67+
it('returns invalid for non-existent directory', () => {
68+
mockFs.existsSync = vi.fn(() => false);
69+
const result = validatePythonEnvironmentInstallDirectory('/nonexistent');
70+
expect(result.valid).toBe(false);
71+
expect(result.message).toMatch(/does not exist/i);
72+
});
73+
74+
it('returns invalid for a file (not directory)', () => {
75+
mockFs.existsSync = vi.fn(() => true);
76+
mockFs.lstatSync = vi.fn(() => ({ isDirectory: () => false } as fs.Stats));
77+
const result = validatePythonEnvironmentInstallDirectory('/some/file.txt');
78+
expect(result.valid).toBe(false);
79+
expect(result.message).toMatch(/not a directory/i);
80+
});
81+
82+
it('returns valid for existing directory', () => {
83+
mockFs.existsSync = vi.fn(() => true);
84+
mockFs.lstatSync = vi.fn(() => ({ isDirectory: () => true } as fs.Stats));
85+
const result = validatePythonEnvironmentInstallDirectory('/valid/dir');
86+
expect(result.valid).toBe(true);
87+
});
88+
89+
it('returns invalid on fs error', () => {
90+
mockFs.existsSync = vi.fn(() => {
91+
throw new Error('permission denied');
92+
});
93+
const result = validatePythonEnvironmentInstallDirectory('/bad/path');
94+
expect(result.valid).toBe(false);
95+
});
96+
});
97+
98+
describe('environmentSatisfiesRequirements', () => {
99+
it('returns true when jupyterlab version satisfies minimum', () => {
100+
const env = {
101+
name: 'test',
102+
path: '/env',
103+
versions: { jupyterlab: '4.4.7' }
104+
} as any;
105+
expect(environmentSatisfiesRequirements(env)).toBe(true);
106+
});
107+
108+
it('returns false when jupyterlab version too old', () => {
109+
const env = {
110+
name: 'test',
111+
path: '/env',
112+
versions: { jupyterlab: '2.9.9' }
113+
} as any;
114+
expect(environmentSatisfiesRequirements(env)).toBe(false);
115+
});
116+
117+
it('returns false when version missing', () => {
118+
const env = {
119+
name: 'test',
120+
path: '/env',
121+
versions: {}
122+
} as any;
123+
expect(environmentSatisfiesRequirements(env)).toBe(false);
124+
});
125+
126+
it('uses custom requirements when provided', () => {
127+
const env = {
128+
name: 'test',
129+
path: '/env',
130+
versions: { mylib: '2.0.0' }
131+
} as any;
132+
const reqs = [
133+
{
134+
name: 'mylib',
135+
moduleName: 'mylib',
136+
commands: ['--version'],
137+
versionRange: new (require('semver').Range)('^2.0.0'),
138+
pipCommand: 'mylib',
139+
condaCommand: 'mylib'
140+
}
141+
];
142+
expect(environmentSatisfiesRequirements(env, reqs)).toBe(true);
143+
});
144+
});
145+
146+
describe('validateCondaChannels', () => {
147+
it('returns valid for standard channel string', () => {
148+
expect(validateCondaChannels('conda-forge defaults').valid).toBe(true);
149+
});
150+
151+
it('returns valid for empty string (no extra channels)', () => {
152+
expect(validateCondaChannels('').valid).toBe(true);
153+
});
154+
155+
it('returns invalid for channels with special chars', () => {
156+
const result = validateCondaChannels('conda-forge; rm -rf /');
157+
expect(result.valid).toBe(false);
158+
});
159+
});

0 commit comments

Comments
 (0)