Skip to content

Commit

Permalink
Merge pull request #30085 from storybookjs/version-non-patch-from-8.5…
Browse files Browse the repository at this point in the history
….0-beta.1

Release: Prerelease 8.5.0-beta.2
  • Loading branch information
shilman authored Dec 18, 2024
2 parents c75bef5 + b4a82cc commit 131d206
Show file tree
Hide file tree
Showing 23 changed files with 864 additions and 347 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.prerelease.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
## 8.5.0-beta.2

- Addon Test: Clear coverage data when starting or watching - [#30072](https://github.com/storybookjs/storybook/pull/30072), thanks @ghengeveld!
- Addon Test: Improve error message on missing coverage package - [#30088](https://github.com/storybookjs/storybook/pull/30088), thanks @JReinhold!
- UI: Fix test provider event handling on startup - [#30083](https://github.com/storybookjs/storybook/pull/30083), thanks @ghengeveld!
- UI: Keep failing stories in the sidebar, disregarding filters - [#30086](https://github.com/storybookjs/storybook/pull/30086), thanks @JReinhold!

## 8.5.0-beta.1

- Addon A11y: Add conditional rendering for a11y violation number in Testing Module - [#30073](https://github.com/storybookjs/storybook/pull/30073), thanks @valentinpalkovic!
Expand Down
9 changes: 8 additions & 1 deletion code/addons/test/src/components/TestProviderRender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -309,14 +309,21 @@ export const TestProviderRender: FC<
href={'/coverage/index.html'}
// @ts-expect-error ListItem doesn't include all anchor attributes in types, but it is an achor element
target="_blank"
aria-label="Open coverage report"
icon={
<TestStatusIcon
percentage={coverageSummary.percentage}
status={coverageSummary.status}
aria-label={`status: ${coverageSummary.status}`}
/>
}
right={coverageSummary.percentage ? `${coverageSummary.percentage}%` : null}
right={
coverageSummary.percentage ? (
<span aria-label={`${coverageSummary.percentage} percent coverage`}>
{coverageSummary.percentage} %
</span>
) : null
}
/>
) : (
<ListItem
Expand Down
117 changes: 64 additions & 53 deletions code/addons/test/src/manager.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,64 +73,75 @@ addons.register(ADDON_ID, (api) => {
},

stateUpdater: (state, update) => {
if (!update.details?.testResults) {
return;
const updated = {
...state,
...update,
details: { ...state.details, ...update.details },
};

if ((!state.running && update.running) || (!state.watching && update.watching)) {
// Clear coverage data when starting test run or enabling watch mode
delete updated.details.coverageSummary;
}

(async () => {
await api.experimental_updateStatus(
TEST_PROVIDER_ID,
Object.fromEntries(
update.details.testResults.flatMap((testResult) =>
testResult.results
.filter(({ storyId }) => storyId)
.map(({ storyId, status, testRunId, ...rest }) => [
storyId,
{
title: 'Component tests',
status: statusMap[status],
description:
'failureMessages' in rest && rest.failureMessages
? rest.failureMessages.join('\n')
: '',
data: { testRunId },
onClick: openTestsPanel,
sidebarContextMenu: false,
} satisfies API_StatusObject,
])
if (update.details?.testResults) {
(async () => {
await api.experimental_updateStatus(
TEST_PROVIDER_ID,
Object.fromEntries(
update.details.testResults.flatMap((testResult) =>
testResult.results
.filter(({ storyId }) => storyId)
.map(({ storyId, status, testRunId, ...rest }) => [
storyId,
{
title: 'Component tests',
status: statusMap[status],
description:
'failureMessages' in rest && rest.failureMessages
? rest.failureMessages.join('\n')
: '',
data: { testRunId },
onClick: openTestsPanel,
sidebarContextMenu: false,
} satisfies API_StatusObject,
])
)
)
)
);
);

await api.experimental_updateStatus(
'storybook/addon-a11y/test-provider',
Object.fromEntries(
update.details.testResults.flatMap((testResult) =>
testResult.results
.filter(({ storyId }) => storyId)
.map(({ storyId, testRunId, reports }) => {
const a11yReport = reports.find((r: any) => r.type === 'a11y');
return [
storyId,
a11yReport
? ({
title: 'Accessibility tests',
description: '',
status: statusMap[a11yReport.status],
data: { testRunId },
onClick: () => {
api.setSelectedPanel('storybook/a11y/panel');
api.togglePanel(true);
},
sidebarContextMenu: false,
} satisfies API_StatusObject)
: null,
];
})
await api.experimental_updateStatus(
'storybook/addon-a11y/test-provider',
Object.fromEntries(
update.details.testResults.flatMap((testResult) =>
testResult.results
.filter(({ storyId }) => storyId)
.map(({ storyId, testRunId, reports }) => {
const a11yReport = reports.find((r: any) => r.type === 'a11y');
return [
storyId,
a11yReport
? ({
title: 'Accessibility tests',
description: '',
status: statusMap[a11yReport.status],
data: { testRunId },
onClick: () => {
api.setSelectedPanel('storybook/a11y/panel');
api.togglePanel(true);
},
sidebarContextMenu: false,
} satisfies API_StatusObject)
: null,
];
})
)
)
)
);
})();
);
})();
}

return updated;
},
} as Addon_TestProviderType<Details, Config>);
}
Expand Down
4 changes: 2 additions & 2 deletions code/addons/test/src/node/coverage-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { TestManager } from './test-manager';

export type StorybookCoverageReporterOptions = {
testManager: TestManager;
coverageOptions: ResolvedCoverageOptions<'v8'>;
coverageOptions: ResolvedCoverageOptions<'v8'> | undefined;
};

export default class StorybookCoverageReporter extends ReportBase implements Partial<Visitor> {
Expand All @@ -32,7 +32,7 @@ export default class StorybookCoverageReporter extends ReportBase implements Par

// Fallback to Vitest's default watermarks https://vitest.dev/config/#coverage-watermarks
const [lowWatermark = 50, highWatermark = 80] =
this.#coverageOptions.watermarks?.statements ?? [];
this.#coverageOptions?.watermarks?.statements ?? [];

const coverageDetails: Details['coverageSummary'] = {
percentage,
Expand Down
9 changes: 1 addition & 8 deletions code/addons/test/src/node/test-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,7 @@ export class TestManager {
coverage: this.coverage,
});
} catch (e) {
const isV8 = e.message?.includes('@vitest/coverage-v8');
const isIstanbul = e.message?.includes('@vitest/coverage-istanbul');

if (e.message?.includes('Error: Failed to load url') && (isIstanbul || isV8)) {
const coveragePackage = isIstanbul ? 'coverage-istanbul' : 'coverage-v8';
e.message = `Please install the @vitest/${coveragePackage} package to run with coverage`;
}
this.reportFatalError('Failed to change coverage mode', e);
this.reportFatalError('Failed to change coverage configuration', e);
}
}
}
Expand Down
16 changes: 11 additions & 5 deletions code/addons/test/src/node/vitest-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class VitestManager {
join(packageDir, 'dist/node/coverage-reporter.js'),
{
testManager: this.testManager,
coverageOptions: this.vitest?.config?.coverage as ResolvedCoverageOptions<'v8'>,
coverageOptions: this.vitest?.config?.coverage as ResolvedCoverageOptions<'v8'> | undefined,
},
];
const coverageOptions = (
Expand Down Expand Up @@ -89,15 +89,21 @@ export class VitestManager {
try {
await this.vitest.init();
} catch (e) {
let message = 'Failed to initialize Vitest';
const isV8 = e.message?.includes('@vitest/coverage-v8');
const isIstanbul = e.message?.includes('@vitest/coverage-istanbul');

if (e.message?.includes('Error: Failed to load url') && (isIstanbul || isV8)) {
if (
(e.message?.includes('Failed to load url') && (isIstanbul || isV8)) ||
// Vitest will sometimes not throw the correct missing-package-detection error, so we have to check for this as well
(e instanceof TypeError &&
e?.message === "Cannot read properties of undefined (reading 'name')")
) {
const coveragePackage = isIstanbul ? 'coverage-istanbul' : 'coverage-v8';
e.message = `Please install the @vitest/${coveragePackage} package to run with coverage`;
message += `\n\nPlease install the @vitest/${coveragePackage} package to collect coverage\n`;
}

this.testManager.reportFatalError('Failed to init Vitest', e);
this.testManager.reportFatalError(message, e);
return;
}

await this.setupWatchers();
Expand Down
59 changes: 59 additions & 0 deletions code/core/src/manager-api/lib/stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { describe, expect, it } from 'vitest';

import type { API_PreparedStoryIndex, StoryIndexV2, StoryIndexV3 } from '@storybook/core/types';

import type { State } from '../root';
import { mockEntries } from '../tests/mockStoriesEntries';
import {
transformStoryIndexToStoriesHash,
transformStoryIndexV2toV3,
transformStoryIndexV3toV4,
transformStoryIndexV4toV5,
Expand Down Expand Up @@ -216,3 +218,60 @@ describe('transformStoryIndexV4toV5', () => {
`);
});
});

describe('transformStoryIndexToStoriesHash', () => {
it('does not apply filters to failing stories', () => {
// Arrange - set up an index with two stories, one of which has a failing status
const indexV5: API_PreparedStoryIndex = {
v: 5,
entries: {
'1': {
id: '1',
type: 'story',
title: 'Story 1',
name: 'Story 1',
importPath: './path/to/story-1.ts',
parameters: {},
tags: [],
},
'2': {
id: '2',
type: 'story',
title: 'Story 2',
name: 'Story 2',
importPath: './path/to/story-2.ts',
parameters: {},
tags: [],
},
},
};

const filters: State['filters'] = {
someFilter: () => false,
};

const status: State['status'] = {
'1': { someStatus: { status: 'error', title: 'broken', description: 'very bad' } },
'2': { someStatus: { status: 'success', title: 'perfect', description: 'nice' } },
};

const options = {
provider: {
getConfig: () => ({ sidebar: {} }),
} as any,
docsOptions: { docsMode: false },
filters,
status,
};

// Act - transform the index to hashes
const result = transformStoryIndexToStoriesHash(indexV5, options);

// Assert - the failing story is still present in the result, even though the filters remove all stories
expect(Object.keys(result)).toHaveLength(2);
expect(result['story-1']).toBeTruthy();
expect(result['1']).toBeTruthy();
expect(result['story-2']).toBeUndefined();
expect(result['2']).toBeUndefined();
});
});
8 changes: 7 additions & 1 deletion code/core/src/manager-api/lib/stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,17 @@ export const transformStoryIndexToStoriesHash = (
const entryValues = Object.values(index.entries).filter((entry: any) => {
let result = true;

// All stories with a failing status should always show up, regardless of the applied filters
const storyStatus = status[entry.id];
if (Object.values(storyStatus ?? {}).some(({ status: s }) => s === 'error')) {
return result;
}

Object.values(filters).forEach((filter: any) => {
if (result === false) {
return;
}
result = filter({ ...entry, status: status[entry.id] });
result = filter({ ...entry, status: storyStatus });
});

return result;
Expand Down
7 changes: 4 additions & 3 deletions code/core/src/manager/components/sidebar/SidebarBottom.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { Fragment, useEffect, useRef, useState } from 'react';
import React, { Fragment, useEffect, useLayoutEffect, useRef, useState } from 'react';

import { styled } from '@storybook/core/theming';
import { type API_FilterFunction, type API_StatusValue } from '@storybook/core/types';
import { type API_FilterFunction } from '@storybook/core/types';

import {
TESTING_MODULE_CRASH_REPORT,
Expand Down Expand Up @@ -119,7 +119,8 @@ export const SidebarBottomBase = ({
api.experimental_setFilter('sidebar-bottom-filter', filter);
}, [api, hasWarnings, hasErrors, warningsActive, errorsActive]);

useEffect(() => {
// Register listeners before the first render
useLayoutEffect(() => {
const onCrashReport = ({ providerId, ...details }: TestingModuleCrashReportPayload) => {
api.updateTestProviderState(providerId, {
error: { name: 'Crashed!', message: details.error.message },
Expand Down
5 changes: 3 additions & 2 deletions code/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
"type-fest": "~2.19"
},
"dependencies": {
"@chromatic-com/storybook": "^3.2.0",
"@chromatic-com/storybook": "^3.2.2",
"@happy-dom/global-registrator": "^14.12.0",
"@nx/eslint": "20.2.2",
"@nx/vite": "20.2.2",
Expand Down Expand Up @@ -294,5 +294,6 @@
"Dependency Upgrades"
]
]
}
},
"deferredNextVersion": "8.5.0-beta.2"
}
Loading

0 comments on commit 131d206

Please sign in to comment.