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: Always run Vitest in watch mode internally #29749

Merged
merged 8 commits into from
Dec 10, 2024
50 changes: 24 additions & 26 deletions code/addons/test/src/node/test-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ export class TestManager {
this.vitestManager.startVitest().then(() => options.onReady?.());
}

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

async handleConfigChange(
Expand All @@ -52,24 +52,28 @@ export class TestManager {
return;
}
if (this.coverage !== payload.config.coverage) {
this.coverage = payload.config.coverage;
try {
this.coverage = payload.config.coverage;
await this.restartVitest({ watchMode: this.watchMode, coverage: this.coverage });
await this.restartVitest({ coverage: this.coverage });
} catch (e) {
this.reportFatalError('Failed to change coverage mode', e);
}
}
}

async handleWatchModeRequest(payload: TestingModuleWatchModeRequestPayload) {
try {
if (payload.providerId !== TEST_PROVIDER_ID) {
return;
}
if (payload.providerId !== TEST_PROVIDER_ID) {
return;
}
this.watchMode = payload.watchMode;

if (this.watchMode !== payload.watchMode) {
this.watchMode = payload.watchMode;
await this.restartVitest({ watchMode: this.watchMode, coverage: false });
try {
if (payload.watchMode && this.coverage) {
// if watch mode is toggled on and coverage is already enabled, restart vitest without coverage to automatically disable it
this.restartVitest({ coverage: false });
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
} else if (!payload.watchMode && this.coverage) {
// if watch mode is toggled off and coverage is already enabled, restart vitest with coverage to automatically re-enable it
this.restartVitest({ coverage: this.coverage });
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
}
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
this.reportFatalError('Failed to change watch mode', e);
Expand All @@ -85,28 +89,22 @@ export class TestManager {
this.coverage = payload.config.coverage;
}

const allTestsRun = (payload.storyIds ?? []).length === 0;
if (this.coverage) {
/*
If we have coverage enabled and we're running all stories,
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.
*/
/*
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.
*/
const temporarilyDisableCoverage = this.coverage && (payload.storyIds ?? []).length === 0;
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
if (temporarilyDisableCoverage) {
await this.restartVitest({
watchMode: allTestsRun ? false : this.watchMode,
coverage: allTestsRun,
coverage: false,
});
}

await this.vitestManager.runTests(payload);

if (this.coverage && !allTestsRun) {
if (temporarilyDisableCoverage) {
// 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.restartVitest({ coverage: this.coverage });
}
} catch (e) {
this.reportFatalError('Failed to run tests', e);
Expand Down
14 changes: 7 additions & 7 deletions code/addons/test/src/node/vitest-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class VitestManager {

constructor(private testManager: TestManager) {}

async startVitest({ watchMode = false, coverage = false } = {}) {
async startVitest({ coverage = false } = {}) {
const { createVitest } = await import('vitest/node');

const storybookCoverageReporter: [string, StorybookCoverageReporterOptions] = [
Expand All @@ -53,7 +53,7 @@ export class VitestManager {
? {
enabled: true,
clean: false,
cleanOnRerun: !watchMode,
cleanOnRerun: false,
Comment on lines 57 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: cleanOnRerun:false could potentially lead to stale coverage data if files are deleted or renamed

reportOnFailure: true,
reporter: [['html', {}], storybookCoverageReporter],
reportsDirectory: resolvePathInStorybookCache(COVERAGE_DIRECTORY),
Expand All @@ -62,9 +62,8 @@ export class VitestManager {
) as CoverageOptions;

this.vitest = await createVitest('test', {
watch: watchMode,
watch: true,
passWithNoTests: false,
changed: watchMode,
// TODO:
// Do we want to enable Vite's default reporter?
// The output in the terminal might be too spamy and it might be better to
Expand All @@ -82,9 +81,7 @@ export class VitestManager {

await this.vitest.init();

if (watchMode) {
await this.setupWatchers();
}
await this.setupWatchers();
}

private updateLastChanged(filepath: string) {
Expand Down Expand Up @@ -276,6 +273,9 @@ export class VitestManager {
this.updateLastChanged(id);
this.storyCountForCurrentRun = 0;

if (!this.testManager.watchMode) {
return;
}
JReinhold marked this conversation as resolved.
Show resolved Hide resolved
await this.runAffectedTests(file);
}

Expand Down
Loading