Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addon Test: Fix run request while booting or restarting Vitest #29829

Merged
merged 9 commits into from
Dec 6, 2024
17 changes: 12 additions & 5 deletions code/addons/test/src/node/boot-test-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,13 @@ const MAX_START_TIME = 30000;
// which is at the root. Then, from the root, we want to load `node/vitest.mjs`
const vitestModulePath = join(__dirname, 'node', 'vitest.mjs');

// Events that were triggered before Vitest was ready are queued up and resent once it's ready
const eventQueue: { type: string; args: any[] }[] = [];

let child: null | ChildProcess;
let ready = false;

const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: any[]) => {
const bootTestRunner = async (channel: Channel) => {
let stderr: string[] = [];

function reportFatalError(e: any) {
Expand Down Expand Up @@ -81,9 +84,10 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a

child.on('message', (result: any) => {
if (result.type === 'ready') {
// Resend the event that triggered the boot sequence, now that the child is ready to handle it
if (initEvent && initArgs) {
child?.send({ type: initEvent, args: initArgs, from: 'server' });
// Resend events that triggered (during) the boot sequence, now that Vitest is ready
while (eventQueue.length) {
const { type, args } = eventQueue.shift();
child?.send({ type, args, from: 'server' });
}
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved

// Forward all events from the channel to the child process
Expand Down Expand Up @@ -129,9 +133,12 @@ const bootTestRunner = async (channel: Channel, initEvent?: string, initArgs?: a
};

export const runTestRunner = async (channel: Channel, initEvent?: string, initArgs?: any[]) => {
if (!ready && initEvent) {
eventQueue.push({ type: initEvent, args: initArgs });
}
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
if (!child) {
ready = false;
await bootTestRunner(channel, initEvent, initArgs);
await bootTestRunner(channel);
ready = true;
}
};
Expand Down
23 changes: 12 additions & 11 deletions code/addons/test/src/node/test-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,6 @@ export class TestManager {
this.vitestManager.startVitest().then(() => options.onReady?.());
}

async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) {
await this.vitestManager.vitest?.runningPromise;
await this.vitestManager.closeVitest();
await this.vitestManager.startVitest({ watchMode, coverage });
}

async handleConfigChange(
payload: TestingModuleConfigChangePayload<{ coverage: boolean; a11y: boolean }>
) {
Expand All @@ -54,7 +48,10 @@ export class TestManager {
if (this.coverage !== payload.config.coverage) {
try {
this.coverage = payload.config.coverage;
await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
await this.vitestManager.restartVitest({
watchMode: this.watchMode,
coverage: this.coverage,
});
} catch (e) {
const isV8 = e.message?.includes('@vitest/coverage-v8');
const isIstanbul = e.message?.includes('@vitest/coverage-istanbul');
Expand All @@ -76,7 +73,7 @@ export class TestManager {

if (this.watchMode !== payload.watchMode) {
this.watchMode = payload.watchMode;
await this.restartVitest({ watchMode: this.watchMode, coverage: false });
await this.vitestManager.restartVitest({ watchMode: this.watchMode, coverage: false });
}
} catch (e) {
this.reportFatalError('Failed to change watch mode', e);
Expand All @@ -99,21 +96,25 @@ export class TestManager {
we have to restart Vitest AND disable watch mode otherwise the coverage report will be incorrect,
Vitest behaves wonky when re-using the same Vitest instance but with watch mode disabled,
among other things it causes the coverage report to be incorrect and stale.

If we're only running a subset of stories, we have to temporarily disable coverage,
as a coverage report for a subset of stories is not useful.
*/
await this.restartVitest({
this.vitestManager.restartVitest({
watchMode: allTestsRun ? false : this.watchMode,
coverage: allTestsRun,
});
}
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved

await this.vitestManager.vitestRestartPromise;
await this.vitestManager.runTests(payload);

if (this.coverage && !allTestsRun) {
// Re-enable coverage if it was temporarily disabled because of a subset of stories was run
await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
await this.vitestManager.restartVitest({
watchMode: this.watchMode,
coverage: this.coverage,
});
}
} catch (e) {
this.reportFatalError('Failed to run tests', e);
Expand Down
26 changes: 23 additions & 3 deletions code/addons/test/src/node/vitest-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ export class VitestManager {

vitestStartupCounter = 0;

vitestRestartPromise: Promise<void> | null = null;

storyCountForCurrentRun: number = 0;

constructor(private testManager: TestManager) {}
Expand Down Expand Up @@ -99,12 +101,28 @@ export class VitestManager {
}
}

async restartVitest({ watchMode, coverage }: { watchMode: boolean; coverage: boolean }) {
await this.vitestRestartPromise;
this.vitestRestartPromise = new Promise(async (resolve, reject) => {
try {
await this.vitest?.runningPromise;
await this.closeVitest();
await this.startVitest({ watchMode, coverage });
resolve();
} catch (e) {
reject(e);
}
});
ghengeveld marked this conversation as resolved.
Show resolved Hide resolved
return this.vitestRestartPromise;
}

private updateLastChanged(filepath: string) {
const projects = this.vitest!.getModuleProjects(filepath);
projects.forEach(({ server, browser }) => {
const serverMods = server.moduleGraph.getModulesByFile(filepath);
serverMods?.forEach((mod) => server.moduleGraph.invalidateModule(mod));

if (server) {
const serverMods = server.moduleGraph.getModulesByFile(filepath);
serverMods?.forEach((mod) => server.moduleGraph.invalidateModule(mod));
}
if (browser) {
const browserMods = browser.vite.moduleGraph.getModulesByFile(filepath);
browserMods?.forEach((mod) => browser.vite.moduleGraph.invalidateModule(mod));
Expand Down Expand Up @@ -148,6 +166,8 @@ export class VitestManager {
async runTests(requestPayload: TestingModuleRunRequestPayload<Config>) {
if (!this.vitest) {
await this.startVitest();
} else {
await this.vitestRestartPromise;
}

this.resetTestNamePattern();
Expand Down
10 changes: 8 additions & 2 deletions code/core/src/manager-api/modules/experimental_testmodule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export const init: ModuleFn<SubAPI, SubState> = ({ store, fullAPI }) => {
clearTestProviderState(id) {
const update = {
cancelling: false,
running: true,
running: false,
failed: false,
crashed: false,
progress: undefined,
Expand All @@ -85,7 +85,13 @@ export const init: ModuleFn<SubAPI, SubState> = ({ store, fullAPI }) => {
runTestProvider(id, options) {
const index = store.getState().index;
invariant(index, 'The index is currently unavailable');
api.updateTestProviderState(id, { running: true, failed: false, crashed: false });

api.updateTestProviderState(id, {
running: true,
failed: false,
crashed: false,
progress: undefined,
});

const provider = store.getState().testProviders[id];

Expand Down
Loading