Skip to content

Commit

Permalink
Merge pull request #30077 from storybookjs/valentin/a11y-show-skipped…
Browse files Browse the repository at this point in the history 
…-tests

Addon A11y: Show skipped label and fix A11y status handling in Testing Module
  • Loading branch information
@valentinpalkovic
valentinpalkovic authored Dec 18, 2024
2 parents 0104e15 + 3cded02 commit 4b23a1a
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 27 deletions.
24 changes: 24 additions & 0 deletions code/addons/test/src/components/TestProviderRender.stories.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ const baseState: TestProviderState<Details, Config> = {
coverage: false,
},
details: {
config: {
a11y: false,
coverage: false,
},
testResults: [
{
endTime: 0,
Expand Down Expand Up @@ -141,6 +145,10 @@ export const WithCoverageNegative: Story = {
...config,
...baseState,
details: {
config: {
a11y: false,
coverage: true,
},
testResults: [],
coverageSummary: {
percentage: 20,
Expand All @@ -162,6 +170,10 @@ export const WithCoverageWarning: Story = {
...baseState,
details: {
testResults: [],
config: {
a11y: false,
coverage: true,
},
coverageSummary: {
percentage: 50,
status: 'warning',
Expand All @@ -182,6 +194,10 @@ export const WithCoveragePositive: Story = {
...baseState,
details: {
testResults: [],
config: {
a11y: false,
coverage: true,
},
coverageSummary: {
percentage: 80,
status: 'positive',
Expand All @@ -206,6 +222,10 @@ export const Editing: Story = {
},
details: {
testResults: [],
config: {
a11y: false,
coverage: false,
},
},
},
},
Expand All @@ -229,6 +249,10 @@ export const EditingAndWatching: Story = {
},
details: {
testResults: [],
config: {
a11y: true,
coverage: true, // should be automatically disabled in the UI
},
},
},
},
Expand Down
29 changes: 22 additions & 7 deletions code/addons/test/src/components/TestProviderRender.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,12 +138,14 @@ export const TestProviderRender: FC<
return 'unknown';
}

if (!a11yResults) {
const definedA11yResults = a11yResults?.filter(Boolean) ?? [];

if (!definedA11yResults || definedA11yResults.length === 0) {
return 'unknown';
}

const failed = a11yResults.some((result) => result?.status === 'failed');
const warning = a11yResults.some((result) => result?.status === 'warning');
const failed = definedA11yResults.some((result) => result?.status === 'failed');
const warning = definedA11yResults.some((result) => result?.status === 'warning');

if (failed) {
return 'negative';
Expand All @@ -154,11 +156,24 @@ export const TestProviderRender: FC<
return 'positive';
}, [state.running, isA11yAddon, config.a11y, a11yResults]);

const a11yNotPassedAmount = a11yResults?.filter(
(result) => result?.status === 'failed' || result?.status === 'warning'
).length;
const a11yNotPassedAmount = state.config?.a11y
? a11yResults?.filter((result) => result?.status === 'failed' || result?.status === 'warning')
.length
: undefined;

const a11ySkippedAmount =
state.running || !state?.details.config?.a11y || !state.config.a11y
? null
: a11yResults?.filter((result) => !result).length;

const a11ySkippedLabel = a11ySkippedAmount
? a11ySkippedAmount === 1 && isStoryEntry
? '(skipped)'
: `(${a11ySkippedAmount} skipped)`
: '';

const storyId = isStoryEntry ? entryId : undefined;

const results = (state.details?.testResults || [])
.flatMap((test) => {
if (!entryId) {
Expand Down Expand Up @@ -333,7 +348,7 @@ export const TestProviderRender: FC<
)}
{isA11yAddon && (
<ListItem
title={<ItemTitle enabled={config.a11y}>Accessibility</ItemTitle>}
title={<ItemTitle enabled={config.a11y}>Accessibility {a11ySkippedLabel}</ItemTitle>}
onClick={
(a11yStatus === 'negative' || a11yStatus === 'warning') && a11yResults.length
? () => {
Expand Down
1 change: 1 addition & 0 deletions code/addons/test/src/constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export interface Config {

export type Details = {
testResults: TestResult[];
config: Config;
coverageSummary?: {
status: 'positive' | 'warning' | 'negative' | 'unknown';
percentage: number;
Expand Down
1 change: 1 addition & 0 deletions code/addons/test/src/node/reporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,7 @@ export class StorybookReporter implements Reporter {
} as TestingModuleProgressReportProgress,
details: {
testResults,
config: this.testManager.config,
},
};
}
Expand Down
14 changes: 7 additions & 7 deletions code/addons/test/src/node/test-manager.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ describe('TestManager', () => {

it('should handle watch mode request', async () => {
const testManager = await TestManager.start(mockChannel, options);
expect(testManager.watchMode).toBe(false);
expect(testManager.config.watchMode).toBe(false);
expect(createVitest).toHaveBeenCalledTimes(1);

await testManager.handleWatchModeRequest({ providerId: TEST_PROVIDER_ID, watchMode: true });
expect(testManager.watchMode).toBe(true);
expect(testManager.config.watchMode).toBe(true);
expect(createVitest).toHaveBeenCalledTimes(1); // shouldn't restart vitest
});

Expand Down Expand Up @@ -149,37 +149,37 @@ describe('TestManager', () => {

it('should handle coverage toggling', async () => {
const testManager = await TestManager.start(mockChannel, options);
expect(testManager.coverage).toBe(false);
expect(testManager.config.coverage).toBe(false);
expect(createVitest).toHaveBeenCalledTimes(1);
createVitest.mockClear();

await testManager.handleConfigChange({
providerId: TEST_PROVIDER_ID,
config: { coverage: true, a11y: false },
});
expect(testManager.coverage).toBe(true);
expect(testManager.config.coverage).toBe(true);
expect(createVitest).toHaveBeenCalledTimes(1);
createVitest.mockClear();

await testManager.handleConfigChange({
providerId: TEST_PROVIDER_ID,
config: { coverage: false, a11y: false },
});
expect(testManager.coverage).toBe(false);
expect(testManager.config.coverage).toBe(false);
expect(createVitest).toHaveBeenCalledTimes(1);
});

it('should temporarily disable coverage on focused tests', async () => {
vitest.globTestSpecs.mockImplementation(() => tests);
const testManager = await TestManager.start(mockChannel, options);
expect(testManager.coverage).toBe(false);
expect(testManager.config.coverage).toBe(false);
expect(createVitest).toHaveBeenCalledTimes(1);

await testManager.handleConfigChange({
providerId: TEST_PROVIDER_ID,
config: { coverage: true, a11y: false },
});
expect(testManager.coverage).toBe(true);
expect(testManager.config.coverage).toBe(true);
expect(createVitest).toHaveBeenCalledTimes(2);

await testManager.handleRunRequest({
Expand Down
30 changes: 19 additions & 11 deletions code/addons/test/src/node/test-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@ import { VitestManager } from './vitest-manager';
export class TestManager {
vitestManager: VitestManager;

watchMode = false;

coverage = false;
config = {
watchMode: false,
coverage: false,
a11y: false,
};

constructor(
private channel: Channel,
Expand All @@ -44,13 +46,19 @@ export class TestManager {
return;
}

const previousConfig = this.config;

this.config = {
...this.config,
...payload.config,
} satisfies Config;

process.env.VITEST_STORYBOOK_CONFIG = JSON.stringify(payload.config);

if (this.coverage !== payload.config.coverage) {
this.coverage = payload.config.coverage;
if (previousConfig.coverage !== payload.config.coverage) {
try {
await this.vitestManager.restartVitest({
coverage: this.coverage,
coverage: this.config.coverage,
});
} catch (e) {
this.reportFatalError('Failed to change coverage configuration', e);
Expand All @@ -62,7 +70,7 @@ export class TestManager {
if (payload.providerId !== TEST_PROVIDER_ID) {
return;
}
this.watchMode = payload.watchMode;
this.config.watchMode = payload.watchMode;

if (payload.config) {
this.handleConfigChange({
Expand All @@ -71,14 +79,14 @@ export class TestManager {
});
}

if (this.coverage) {
if (this.config.coverage) {
try {
if (payload.watchMode) {
// if watch mode is toggled on and coverage is already enabled, restart vitest without coverage to automatically disable it
await this.vitestManager.restartVitest({ coverage: false });
} else {
// if watch mode is toggled off and coverage is already enabled, restart vitest with coverage to automatically re-enable it
await this.vitestManager.restartVitest({ coverage: this.coverage });
await this.vitestManager.restartVitest({ coverage: this.config.coverage });
}
} catch (e) {
this.reportFatalError('Failed to change watch mode while coverage was enabled', e);
Expand All @@ -104,7 +112,7 @@ export class TestManager {
as a coverage report for a subset of stories is not useful.
*/
const temporarilyDisableCoverage =
this.coverage && !this.watchMode && (payload.storyIds ?? []).length > 0;
this.config.coverage && !this.config.watchMode && (payload.storyIds ?? []).length > 0;
if (temporarilyDisableCoverage) {
await this.vitestManager.restartVitest({
coverage: false,
Expand All @@ -117,7 +125,7 @@ export class TestManager {

if (temporarilyDisableCoverage) {
// Re-enable coverage if it was temporarily disabled because of a subset of stories was run
await this.vitestManager.restartVitest({ coverage: this.coverage });
await this.vitestManager.restartVitest({ coverage: this.config.coverage });
}
} catch (e) {
this.reportFatalError('Failed to run tests', e);
Expand Down
4 changes: 2 additions & 2 deletions code/addons/test/src/node/vitest-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ export class VitestManager {
this.filterStories(story, spec.moduleId, { include, exclude, skip })
);
if (matches.length) {
if (!this.testManager.watchMode) {
if (!this.testManager.config.watchMode) {
// Clear the file cache if watch mode is not enabled
this.updateLastChanged(spec.moduleId);
}
Expand Down Expand Up @@ -320,7 +320,7 @@ export class VitestManager {

// when watch mode is disabled, don't trigger any tests (below)
// but still invalidate the cache for the changed file, which is handled above
if (!this.testManager.watchMode) {
if (!this.testManager.config.watchMode) {
return;
}
await this.runAffectedTests(file);
Expand Down

0 comments on commit 4b23a1a

Please sign in to comment.