test: main-process unit coverage with CI-enforced thresholds (C2) (#967)

* test: cover token enum maps and event type registries

EnvironmentTypeName mapping plus IEventTypeMain/Renderer registry shape.

Note: AI-assisted (Claude Code). Manually verified: 2-dot import paths, vitest green, no tautological asserts (enum-completeness loop pairs toBeDefined with a typeof string check).

* test: cover main-process utils helpers

Path/home resolution, port checks, env path builders, wait helpers.

Note: AI-assisted (Claude Code). Manually verified: rewrote getRelativePathToUserHome from a typeof-function tautology to a behavioral assertion, mutation check (return absolutePath instead of the ~ form) fails the test and restoring passes.

* test: cover settings, workspace settings and appdata

Default values, setValue overrides, persisted application data shape.

Note: AI-assisted (Claude Code). Manually verified: 2-dot paths, removed unused imports, made the workspace override test assert the pre-override default differs from the new value, vitest green.

* test: cover SessionConfig creation from CLI args

Remote vs local sessions, working dir resolution, python path inclusion.

Note: AI-assisted (Claude Code). Manually verified: 2-dot paths, the toBeDefined matchers are each paired with concrete assertions on isRemote/remoteURL/pythonPath, vitest green.

* test: cover CLI arg parsing and command handlers

parseCLIArgs subcommands and the env list/handler paths.

Note: AI-assisted (Claude Code). Manually verified: switching to 2-dot paths made the appdata mock actually apply, which exposed a stale mock field (discoveredPythonPaths vs discoveredPythonEnvs); fixed the mock to match the real shape, vitest green.

* test: cover EventManager registration and dispose

Sync and async handler registration, dispose unregistering all handlers.

Restore ipcMain.removeListener on the electron mock; eventmanager.ts uses
it in unregisterAllEventHandlers and the mock had dropped it.

Note: AI-assisted (Claude Code). Manually verified: dispose tests failed with "removeListener is not a function" until the mock was restored, then 305/305 green.

* test: cover env activate invalid-directory guard

handleEnvActivateCommand was the one cli handler with no coverage; a
mutation (flipping the directory && guard to ||) survived. Adds the
negative path: an invalid environment directory logs an error and does
not activate.

Note: AI-assisted (Claude Code). Manually verified: test passes on the real code, and the guard mutation now fails it (caught), full suite 306/306 green.

* test: convert enum and event-type checks to table-driven it.each

Collapses the repeated per-value it() blocks for IEnvironmentType,
EnvironmentTypeName, PythonEnvResolveErrorType and the EventType
registries into it.each tables, and shares the non-empty / no-duplicate
invariants across both registries via describe.each. Same assertions,
less duplication, easier to extend a row.

Note: AI-assisted (Claude Code). Manually verified: vitest green (the wire-value rows still fail if an enum string is changed), eslint and prettier clean.

* test: gate the unit-tested logic layer with coverage thresholds

Scopes v8 coverage to the main-process logic modules that unit tests can
reach (env, cli, utils, eventmanager, eventtypes, tokens, config), with
all: true so untested branches count. The window/view/preload UI and the
process-entry modules are integration surfaces left to the e2e suite, so
they are not folded into the denominator.

Adds a layer-wide floor plus per-file locks on the well-covered modules
(eventmanager, sessionconfig, settings, appdata) so they cannot silently
regress. Adds a test:coverage script and ignores the coverage output dir.

Note: AI-assisted (Claude Code). Manually verified: measured each file's current numbers first, set thresholds a few points below to leave headroom, yarn test:coverage exits 0 against them.

* test: close mutation-survivor gaps before opening the PR

A mutation sweep over the logic layer found three weak spots where a real
bug would not have failed any test:

- settings.ts: the Setting / UserSettings classes were never constructed,
  so the value getter and the differentThanDefault save filter were
  unasserted. Add round-trip, default, and save-time filtering tests.
- sessionconfig.ts: createFromArgs never asserted the remote persist flag
  or the persist:/partition: prefix. Add both directions.
- appdata.ts: the 20-entry recents cap was uncovered. Add a 25-insert test
  asserting the cap.

Note: AI-assisted (Claude Code). Manually verified: each new test fails when the corresponding source conditional is mutated (value-getter swap, differentThanDefault flip, persistSessionData ===/!==, recents loop bound and MAX) and passes on the real code; full suite 317/317.

* test: ratchet coverage thresholds to the improved levels

The mutation-gap tests lifted settings, sessionconfig and appdata above
their previous floors. Lock the new levels (and bump the layer-wide floor)
so the gains cannot silently regress.

Note: AI-assisted (Claude Code). Manually verified: yarn test:coverage exits 0 against the raised thresholds.

* test: address pre-PR review on mock isolation and weak assertions

Restore spies after each cli-handler test so a stubbed console or
process in one case cannot leak into the next; reset the fs mock at
the top of the utils suite for the same reason. Replace two vacuous
assertions (toBeDefined, not.toThrow) with concrete checks on the
resolved file path and on Number.isNaN of the parsed date. Drop the
decorative comment headers per repo writing conventions.

Note: AI-assisted (Claude Code). Manually verified: 317/317 unit tests
pass, tsc --noEmit clean, eslint clean on the three files, coverage
thresholds hold (lines 50.4 >= 48 floor); mutation-checked the two
strengthened assertions (filesToOpen push removed = 1 fail; isDarkTheme
fallback return false = 1 fail).

* ci: enforce coverage thresholds in the unit-test job

The C2 per-file coverage locks only gate when vitest collects coverage;
`vitest run` alone ignores them. Switch the CI test step to
`yarn test:coverage` (vitest run --coverage) so a regression below any
per-file or global threshold fails the PR check instead of passing
silently. Add timeout-minutes: 10 so the job cannot hang on the default
360-minute runner budget.

Note: AI-assisted (Claude Code). Manually verified: yarn test:coverage
exits 0 at current coverage (317/317); confirmed the gate is live by
raising the global lines threshold to 99 and observing exit 1 with
"does not meet global threshold", then reverting. YAML parses clean.

* fix: correct two main-process bugs surfaced by the new unit tests

deletePythonEnvironment rejected when the target was not installed by
Desktop but did not return, so execution fell through to
fs.rmSync(envPath, { recursive, force }) and deleted the directory the
guard had just refused. Add the missing return. The pre-existing test
only asserted the rejected promise, never that rmSync stayed untouched,
so it passed against the bug; tighten it to assert rmSync is not called.

EventManager registered sync handlers with ipcMain.handle but removed
them with ipcMain.removeListener. Per the Electron ipcMain docs,
handle() handlers are removed with removeHandler(channel); removeListener
only unbinds on() listeners, so sync handlers were never actually
unregistered (dispose left them live). Switch the sync removal paths to
removeHandler and update the tests, which had mirrored the buggy call.

Note: AI-assisted (Claude Code). Manually verified against the Electron
ipcMain docs (removeHandler vs removeListener). Mutation-checked both
fixes: dropping the return makes the rmSync assertion fail; reverting to
removeListener makes the sync-removal assertions fail. 318/318 pass, tsc
and eslint clean, coverage thresholds hold.

* fix: harden promise settling and tighten setting-key assertions

A repo-wide pass for the same bug class the review surfaced turned up
three more spots:

- clearSession wrapped Promise.all in a try/catch but the .then had no
  rejection handler, so an async failure in any clear*() call left the
  returned promise pending forever. Add a .catch that rejects. New test
  asserts it rejects rather than hangs when a clear call rejects.
- runCommand called reject(error) without returning, then fell through
  to resolve(stdout). First settle wins so behaviour was correct, but
  the stray second settle is a footgun if the branch order changes; add
  the return.
- validateCondaPath resolved on a valid conda_version without returning,
  falling through to returnInvalid (a second resolve). Same first-wins
  situation; add the return.

Also replaced expect.any(String) with the specific SettingType key in
the cli set-* handler tests, so writing the wrong setting key would now
fail instead of passing against any string.

Note: AI-assisted (Claude Code). Manually verified: 320/320 pass, tsc
and eslint clean, prettier formatted. Mutation-checked clearSession
(removing the .catch makes the new "does not hang" test fail). The two
return additions are no-observable-change hardening (first settle
already won), so they carry no new test.

* test: replace remaining loose assertions with concrete checks

Tighten the cli set-* handler error-path tests to assert the exact
console.error message instead of bare toHaveBeenCalled(), so a reworded
or wrong diagnostic would now fail. Assert the exact userSetPythonEnvs
entry name (venv:/conda: prefix + basename) rather than toContain. Turn
the appData.read "no file" case from a sole not.toThrow into an
observable no-op check: readFileSync is not called and pythonPath is
unchanged.

Note: AI-assisted (Claude Code). Manually verified: 320/320 pass, eslint
clean, prettier formatted. Mutation-checked one tightened message
(rewording the conda no-path error makes its test fail).

* fix: make clearSession best-effort so teardown is never blocked

The previous fix changed clearSession from hanging to rejecting on a
failed clear*(), but several callers await it without a try/catch and
then run required cleanup (connect.ts closes the window and rejects the
connect promise on timeout). A rejection there would skip window.close()
and leave the connect promise pending. Switch to Promise.allSettled so
clearSession always resolves, logging any individual failure. This keeps
the no-hang property and removes the new reject-skips-cleanup risk. The
test now asserts it resolves and logs rather than rejects.

Note: AI-assisted (Claude Code). Manually verified: 320/320 pass, tsc and
eslint clean. Inspected all callers (app.ts, connect.ts, appdata.ts,
sessionwindow.ts, labview.ts); connect.ts awaits without try/catch, which
is why always-resolve is the correct contract. Mutation-checked: reverting
allSettled to Promise.all without a catch makes the teardown test fail.

Author: notluquis

.github/workflows/publish.yml

@@ -16,6 +16,7 @@ jobs:
   test:
     name: 'Unit tests'
     runs-on: ubuntu-latest
+    timeout-minutes: 10
     permissions:
       contents: read
     steps:
@@ -25,7 +26,7 @@ jobs:
           node-version: '20.x'
           cache: 'yarn'
       - run: yarn install --frozen-lockfile
-      - run: yarn test:unit
+      - run: yarn test:coverage
 
   publish:
     needs: test

.gitignore

@@ -7,6 +7,7 @@ node_modules
 .npmrc
 .cache
 .eslintcache
+coverage
 .vscode/*
 !.vscode/launch.json
 *.py[co]

package.json

@@ -8,6 +8,7 @@
     "test": "yarn test:unit",
     "test:unit": "vitest run",
     "test:unit:watch": "vitest",
+    "test:coverage": "vitest run --coverage",
     "test:e2e": "playwright test",
     "clean": "rimraf build dist",
     "watch:tsc": "tsc -w",

src/main/env.ts

@@ -349,6 +349,7 @@ export async function validateCondaPath(
                         resolve({
                           valid: true
                         });
+                        return;
                       }
                     } catch (error) {
                       //

src/main/eventmanager.ts

@@ -67,7 +67,9 @@ export class EventManager implements IDisposable {
     const handlers = this._syncEventHandlers.get(eventType);
     const index = handlers.indexOf(handler);
     if (index !== -1) {
-      ipcMain.removeListener(eventType, handler);
+      // sync handlers register via ipcMain.handle, which is unbound with
+      // removeHandler(channel); removeListener only applies to ipcMain.on.
+      ipcMain.removeHandler(eventType);
       handlers.splice(index, 1);
     }
   }
@@ -85,13 +87,9 @@ export class EventManager implements IDisposable {
   }
 
   unregisterAllSyncEventHandlers() {
-    this._syncEventHandlers.forEach(
-      (handlers: SyncEventHandlerMain[], eventType: EventTypeMain) => {
-        for (const handler of handlers) {
-          ipcMain.removeListener(eventType, handler);
-        }
-      }
-    );
+    for (const eventType of this._syncEventHandlers.keys()) {
+      ipcMain.removeHandler(eventType);
+    }
 
     this._syncEventHandlers.clear();
   }

src/main/utils.ts

@@ -117,18 +117,19 @@ export function isDarkTheme(themeType: string) {
 }
 
 export function clearSession(session: Electron.Session): Promise<void> {
-  return new Promise((resolve, reject) => {
-    try {
-      Promise.all([
-        session.clearCache(),
-        session.clearAuthCache(),
-        session.clearStorageData(),
-        session.flushStorageData()
-      ]).then(() => {
-        resolve();
-      });
-    } catch (error) {
-      reject();
+  // best-effort teardown: callers await this before closing windows, so a
+  // failure to clear one cache must not reject and skip that cleanup, nor hang
+  // (Promise.all with no catch would). allSettled always resolves; log failures.
+  return Promise.allSettled([
+    session.clearCache(),
+    session.clearAuthCache(),
+    session.clearStorageData(),
+    session.flushStorageData()
+  ]).then(results => {
+    for (const result of results) {
+      if (result.status === 'rejected') {
+        log.error('Failed to clear part of the session', result.reason);
+      }
     }
   });
 }
@@ -407,6 +408,9 @@ export async function deletePythonEnvironment(
         'Environment cannot be deleted since it was not installed by JupyterLab Desktop.'
       );
       reject();
+      // without this return the guard is advisory only: execution falls
+      // through and rmSync deletes the directory the guard just refused.
+      return;
     }
 
     try {
@@ -627,6 +631,7 @@ export async function runCommand(
     execFile(executablePath, commands, options, (error, stdout, stderr) => {
       if (error) {
         reject(error);
+        return;
       }
       if (stdout) {
         resolve(stdout);

test/setup/electron-mock.ts

@@ -17,6 +17,7 @@ vi.mock('electron', () => ({
     on: vi.fn(),
     handle: vi.fn(),
     removeHandler: vi.fn(),
+    removeListener: vi.fn(),
     removeAllListeners: vi.fn(),
     emit: vi.fn()
   },