Skip to content

Commit

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

Release: Prerelease 8.5.0-beta.1
  • Loading branch information
vanessayuenn authored Dec 16, 2024
2 parents 88b60be + ba9d402 commit 862c699
Show file tree
Hide file tree
Showing 29 changed files with 356 additions and 164 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.prerelease.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 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!
- Addon A11y: Remove warnings API - [#30049](https://github.com/storybookjs/storybook/pull/30049), thanks @kasperpeulen!
- Addon A11y: Show errors of axe properly - [#30050](https://github.com/storybookjs/storybook/pull/30050), thanks @kasperpeulen!
- Addon Test: Fix printing null% for coverage - [#30061](https://github.com/storybookjs/storybook/pull/30061), thanks @ghengeveld!
- Telemetry: Add metadata distinguishing "apps" from "design systems" - [#30070](https://github.com/storybookjs/storybook/pull/30070), thanks @tmeasday!

## 8.5.0-beta.0

- Automigration: Improve setup file transformation and version range handling for a11y migration - [#30060](https://github.com/storybookjs/storybook/pull/30060), thanks @valentinpalkovic!
Expand Down
6 changes: 1 addition & 5 deletions code/.storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -358,9 +358,5 @@ export const parameters = {
opacity: 0.4,
},
},
a11y: {
warnings: ['minor', 'moderate', 'serious', 'critical'],
},
tags: ['test', 'vitest', '!a11ytest'],
};

export const tags = ['test', 'vitest'];
6 changes: 5 additions & 1 deletion code/addons/a11y/src/components/A11YPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ export const A11YPanel: React.FC = () => {
<>
The accessibility scan encountered an error.
<br />
{typeof error === 'string' ? error : JSON.stringify(error)}
{typeof error === 'string'
? error
: error instanceof Error
? error.toString()
: JSON.stringify(error)}
</>
)}
</Centered>
Expand Down
3 changes: 0 additions & 3 deletions code/addons/a11y/src/params.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,11 @@ export interface Setup {
options: RunOptions;
}

type Impact = NonNullable<ImpactValue>;

export interface A11yParameters {
element?: ElementContext;
config?: Spec;
options?: RunOptions;
/** @deprecated Use globals.a11y.manual instead */
manual?: boolean;
disable?: boolean;
warnings?: Impact[];
}
54 changes: 0 additions & 54 deletions code/addons/a11y/src/preview.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -156,60 +156,6 @@ describe('afterEach', () => {
});
});

it('should report warning status when there are only warnings', async () => {
const context = createContext({
parameters: {
a11y: {
warnings: ['minor'],
},
},
});
const result = {
violations: [
{ impact: 'minor', nodes: [] },
{ impact: 'critical', nodes: [] },
],
};
mockedRun.mockResolvedValue(result as any);

await expect(async () => experimental_afterEach(context)).rejects.toThrow();

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);
expect(context.reporting.addReport).toHaveBeenCalledWith({
type: 'a11y',
version: 1,
result,
status: 'failed',
});
});

it('should report error status when there are warnings and errors', async () => {
const context = createContext({
parameters: {
a11y: {
warnings: ['minor'],
},
},
});
const result = {
violations: [
{ impact: 'minor', nodes: [] },
{ impact: 'critical', nodes: [] },
],
};
mockedRun.mockResolvedValue(result as any);

await expect(async () => experimental_afterEach(context)).rejects.toThrow();

expect(mockedRun).toHaveBeenCalledWith(context.parameters.a11y);
expect(context.reporting.addReport).toHaveBeenCalledWith({
type: 'a11y',
version: 1,
result,
status: 'failed',
});
});

it('should run accessibility checks if "a11ytest" flag is not available and is not running in Vitest', async () => {
const context = createContext({
tags: [],
Expand Down
9 changes: 2 additions & 7 deletions code/addons/a11y/src/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const experimental_afterEach: AfterEach<any> = async ({
}) => {
const a11yParameter: A11yParameters | undefined = parameters.a11y;
const a11yGlobals = globals.a11y;
const warnings = a11yParameter?.warnings ?? [];

const shouldRunEnvironmentIndependent =
a11yParameter?.manual !== true &&
Expand All @@ -38,15 +37,11 @@ export const experimental_afterEach: AfterEach<any> = async ({
if (result) {
const hasViolations = (result?.violations.length ?? 0) > 0;

const hasErrors = result?.violations.some(
(violation) => !warnings.includes(violation.impact!)
);

reporting.addReport({
type: 'a11y',
version: 1,
result: result,
status: hasErrors ? 'failed' : hasViolations ? 'warning' : 'passed',
status: hasViolations ? 'failed' : 'passed',
});

/**
Expand All @@ -58,7 +53,7 @@ export const experimental_afterEach: AfterEach<any> = async ({
* implement proper try catch handling.
*/
if (getIsVitestStandaloneRun()) {
if (hasErrors) {
if (hasViolations) {
// @ts-expect-error - todo - fix type extension of expect from @storybook/test
expect(result).toHaveNoViolations();
}
Expand Down
8 changes: 5 additions & 3 deletions code/addons/test/src/components/TestProviderRender.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ export const TestProviderRender: FC<
state.config || { a11y: false, coverage: false }
);

const isStoryEntry = entryId?.includes('--') ?? false;

const a11yResults = useMemo(() => {
if (!isA11yAddon) {
return [];
Expand Down Expand Up @@ -156,7 +158,7 @@ export const TestProviderRender: FC<
(result) => result?.status === 'failed' || result?.status === 'warning'
).length;

const storyId = entryId?.includes('--') ? entryId : undefined;
const storyId = isStoryEntry ? entryId : undefined;
const results = (state.details?.testResults || [])
.flatMap((test) => {
if (!entryId) {
Expand Down Expand Up @@ -314,7 +316,7 @@ export const TestProviderRender: FC<
aria-label={`status: ${coverageSummary.status}`}
/>
}
right={`${coverageSummary.percentage}%`}
right={coverageSummary.percentage ? `${coverageSummary.percentage}%` : null}
/>
) : (
<ListItem
Expand All @@ -340,7 +342,7 @@ export const TestProviderRender: FC<
: null
}
icon={<TestStatusIcon status={a11yStatus} aria-label={`status: ${a11yStatus}`} />}
right={a11yNotPassedAmount || null}
right={isStoryEntry ? null : a11yNotPassedAmount || null}
/>
)}
</Extras>
Expand Down
1 change: 1 addition & 0 deletions code/core/src/__mocks__/page.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// empty file only matched on path
1 change: 1 addition & 0 deletions code/core/src/__mocks__/path/to/Screens/index.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// empty file only matched on path
2 changes: 1 addition & 1 deletion code/core/src/manager-api/modules/layout.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export const defaultLayoutState: SubState = {
panelPosition: 'bottom',
showTabs: true,
},
selectedPanel: undefined,
selectedPanel: 'chromaui/addon-visual-tests/panel',
theme: create(),
};

Expand Down
2 changes: 1 addition & 1 deletion code/core/src/manager/components/panel/Panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ export const AddonPanel = React.memo<{
return (
<Tabs
absolute={absolute}
{...(selectedPanel ? { selected: selectedPanel } : {})}
{...(selectedPanel && panels[selectedPanel] ? { selected: selectedPanel } : {})}
menuName="Addons"
actions={actions}
showToolsWhenEmpty
Expand Down
71 changes: 71 additions & 0 deletions code/core/src/telemetry/exec-command-count-lines.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import type { Transform } from 'node:stream';
import { PassThrough } from 'node:stream';

import { beforeEach, describe, expect, it, vitest } from 'vitest';

// eslint-disable-next-line depend/ban-dependencies
import { execaCommand as rawExecaCommand } from 'execa';

import { execCommandCountLines } from './exec-command-count-lines';

vitest.mock('execa');

const execaCommand = vitest.mocked(rawExecaCommand);
beforeEach(() => {
execaCommand.mockReset();
});

type ExecaStreamer = typeof Promise & {
stdout: Transform;
kill: () => void;
};

function createExecaStreamer() {
let resolver: () => void;
const promiseLike: ExecaStreamer = new Promise<void>((aResolver, aRejecter) => {
resolver = aResolver;
}) as any;

promiseLike.stdout = new PassThrough();
// @ts-expect-error technically it is invalid to use resolver "before" it is assigned (but not really)
promiseLike.kill = resolver;
return promiseLike;
}

describe('execCommandCountLines', () => {
it('counts lines, many', async () => {
const streamer = createExecaStreamer();
execaCommand.mockReturnValue(streamer as any);

const promise = execCommandCountLines('some command');

streamer.stdout.write('First line\n');
streamer.stdout.write('Second line\n');
streamer.kill();

expect(await promise).toEqual(2);
});

it('counts lines, one', async () => {
const streamer = createExecaStreamer();
execaCommand.mockReturnValue(streamer as any);

const promise = execCommandCountLines('some command');

streamer.stdout.write('First line\n');
streamer.kill();

expect(await promise).toEqual(1);
});

it('counts lines, none', async () => {
const streamer = createExecaStreamer();
execaCommand.mockReturnValue(streamer as any);

const promise = execCommandCountLines('some command');

streamer.kill();

expect(await promise).toEqual(0);
});
});
35 changes: 35 additions & 0 deletions code/core/src/telemetry/exec-command-count-lines.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { createInterface } from 'node:readline';

// eslint-disable-next-line depend/ban-dependencies
import { execaCommand } from 'execa';

/**
* Execute a command in the local terminal and count the lines in the result
*
* @param command The command to execute.
* @param options Execa options
* @returns The number of lines the command returned
*/
export async function execCommandCountLines(
command: string,
options?: Parameters<typeof execaCommand>[1]
) {
const process = execaCommand(command, { shell: true, buffer: false, ...options });
if (!process.stdout) {
// eslint-disable-next-line local-rules/no-uncategorized-errors
throw new Error('Unexpected missing stdout');
}

let lineCount = 0;
const rl = createInterface(process.stdout);
rl.on('line', () => {
lineCount += 1;
});

// If the process errors, this will throw
await process;

rl.close();

return lineCount;
}
14 changes: 14 additions & 0 deletions code/core/src/telemetry/get-application-file-count.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { join } from 'node:path';

import { describe, expect, it } from 'vitest';

import { getApplicationFilesCountUncached } from './get-application-file-count';

const mocksDir = join(__dirname, '..', '__mocks__');

describe('getApplicationFilesCount', () => {
it('should find files with correct names', async () => {
const files = await getApplicationFilesCountUncached(mocksDir);
expect(files).toMatchInlineSnapshot(`2`);
});
});
32 changes: 32 additions & 0 deletions code/core/src/telemetry/get-application-file-count.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { sep } from 'node:path';

import { execCommandCountLines } from './exec-command-count-lines';
import { runTelemetryOperation } from './run-telemetry-operation';

// We are looking for files with the word "page" or "screen" somewhere in them with these exts
const nameMatches = ['page', 'screen'];
const extensions = ['js', 'jsx', 'ts', 'tsx'];

export const getApplicationFilesCountUncached = async (basePath: string) => {
const bothCasesNameMatches = nameMatches.flatMap((match) => [
match,
[match[0].toUpperCase(), ...match.slice(1)].join(''),
]);

const globs = bothCasesNameMatches.flatMap((match) =>
extensions.map((extension) => `"${basePath}${sep}*${match}*.${extension}"`)
);

try {
const command = `git ls-files -- ${globs.join(' ')}`;
return await execCommandCountLines(command);
} catch {
return undefined;
}
};

export const getApplicationFileCount = async (path: string) => {
return runTelemetryOperation('applicationFiles', async () =>
getApplicationFilesCountUncached(path)
);
};
29 changes: 29 additions & 0 deletions code/core/src/telemetry/get-has-router-package.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { expect, it } from 'vitest';

import { getHasRouterPackage } from './get-has-router-package';

it('returns true if there is a routing package in package.json', () => {
expect(
getHasRouterPackage({
dependencies: {
react: '^18',
'react-dom': '^18',
'react-router': '^6',
},
})
).toBe(true);
});

it('returns false if there is a routing package in package.json dependencies', () => {
expect(
getHasRouterPackage({
dependencies: {
react: '^18',
'react-dom': '^18',
},
devDependencies: {
'react-router': '^6',
},
})
).toBe(false);
});
Loading

0 comments on commit 862c699

Please sign in to comment.