Skip to content

Commit

Permalink
Merge pull request #19777 from storybookjs/valentin/fix-tests-on-windows
Browse files Browse the repository at this point in the history
Fix tests on Windows
  • Loading branch information
IanVS authored Nov 15, 2022
2 parents db105ee + 85d3180 commit 82dc644
Show file tree
Hide file tree
Showing 9 changed files with 45 additions and 59 deletions.
16 changes: 6 additions & 10 deletions code/jest.config.js
Original file line number Diff line number Diff line change
@@ -1,21 +1,17 @@
const os = require('os');

// TODO Revisit this test later, when we have a windows machine @valentinpalkovic
/**
* TODO: Some windows related tasks are still commented out, because they are behaving differently on
* a local Windows machine compared to the Windows Server 2022 machine running in GitHub Actions.
* The main issue is that path.sep is behaving differently on the two machines. Some more investagations
* are necessary!
* */
const skipOnWindows = [
'lib/core-server/src/utils/stories-json.test.ts',
'lib/core-server/src/utils/StoryIndexGenerator.test.ts',
'lib/cli/src/helpers.test.ts',
'lib/core-server/src/utils/__tests__/server-statics.test.ts',
'lib/core-common/src/utils/__tests__/template.test.ts',
'addons/storyshots/storyshots-core/src/frameworks/configure.test.ts',
'lib/core-common/src/utils/__tests__/interpret-files.test.ts',
'lib/builder-manager/src/utils/files.test.ts',
'lib/cli/src/helpers.test.ts',
'lib/core-server/src/utils/__tests__/server-statics.test.ts',
'lib/core-common/src/utils/__tests__/template.test.ts',
'addons/storyshots/storyshots-core/src/frameworks/configure.test.ts',
'lib/core-common/src/utils/__tests__/interpret-files.test.ts',
'lib/builder-manager/src/utils/files.test.ts',
];

module.exports = {
Expand Down
18 changes: 14 additions & 4 deletions code/lib/builder-manager/src/utils/files.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
import { platform } from 'os';
import { sanitizePath } from './files';

const os = platform();
const isWindows = os === 'win32';

test('sanitizePath', () => {
const addonsDir = '/Users/username/Projects/projectname/storybook';
const addonsDir = isWindows
? 'C:\\Users\\username\\Projects\\projectname\\storybook'
: '/Users/username/Projects/projectname/storybook';
const text = 'demo text';
const file = {
path: '/Users/username/Projects/projectname/storybook/node_modules/@storybook/addon-x+y/dist/manager.mjs',
path: isWindows
? 'C:\\Users\\username\\Projects\\projectname\\storybook\\node_modules\\@storybook\\addon-x+y\\dist\\manager.mjs'
: '/Users/username/Projects/projectname/storybook/node_modules/@storybook/addon-x+y/dist/manager.mjs',
contents: Uint8Array.from(Array.from(text).map((letter) => letter.charCodeAt(0))),
text,
};
const { location, url } = sanitizePath(file, addonsDir);

expect(location).toMatchInlineSnapshot(
`"/Users/username/Projects/projectname/storybook/node_modules/@storybook/addon-x+y/dist/manager.mjs"`
expect(location).toEqual(
isWindows
? 'C:\\Users\\username\\Projects\\projectname\\storybook\\node_modules\\@storybook\\addon-x+y\\dist\\manager.mjs'
: '/Users/username/Projects/projectname/storybook/node_modules/@storybook/addon-x+y/dist/manager.mjs'
);
expect(url).toMatchInlineSnapshot(
`"./sb-addons/node_modules/%40storybook/addon-x%2By/dist/manager.mjs"`
Expand Down
5 changes: 3 additions & 2 deletions code/lib/builder-manager/src/utils/files.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type { OutputFile } from 'esbuild';
import fs from 'fs-extra';
import { join } from 'path';
import { join, normalize } from 'path';
import slash from 'slash';
import type { Compilation } from '../types';

Expand All @@ -26,7 +26,8 @@ export async function readOrderedFiles(

export function sanitizePath(file: OutputFile, addonsDir: string) {
const filePath = file.path.replace(addonsDir, '');
const location = join(addonsDir, filePath);
const location = normalize(join(addonsDir, filePath));
const url = `./sb-addons${slash(filePath).split('/').map(encodeURIComponent).join('/')}`;

return { location, url };
}
5 changes: 4 additions & 1 deletion code/lib/cli/src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ export async function copyComponents(
const componentsPath = async () => {
const baseDir = getRendererDir(renderer);
const assetsDir = join(baseDir, 'template/cli');

const assetsLanguage = join(assetsDir, languageFolderMapping[language]);
const assetsJS = join(assetsDir, languageFolderMapping[SupportedLanguage.JAVASCRIPT]);
const assetsTSLegacy = join(
Expand Down Expand Up @@ -232,7 +233,9 @@ export async function copyComponents(
};

const destinationPath = await targetPath();
await fse.copy(join(getCliDir(), 'rendererAssets/common'), destinationPath, { overwrite: true });
await fse.copy(join(getCliDir(), 'rendererAssets/common'), destinationPath, {
overwrite: true,
});
await fse.copy(await componentsPath(), destinationPath, { overwrite: true });
}

Expand Down
6 changes: 3 additions & 3 deletions code/lib/core-client/src/PreviewWeb.mockdata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ export const waitForEvents = (
events.forEach((event) => mockChannel.on(event, listener));

// Don't wait too long
waitForQuiescence().then(() =>
reject(new Error(`Event was not emitted in time: ${debugLabel || events}`))
);
waitForQuiescence().then(() => {
reject(new Error(`Event was not emitted in time: ${debugLabel || events}`));
});
});
};

Expand Down
4 changes: 2 additions & 2 deletions code/lib/core-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@
"util-deprecate": "^1.0.2"
},
"devDependencies": {
"@types/mock-fs": "^4.13.0",
"@types/mock-fs": "^4.13.1",
"@types/picomatch": "^2.3.0",
"mock-fs": "^4.13.0",
"mock-fs": "^5.2.0",
"type-fest": "^2.17.0",
"typescript": "~4.6.3"
},
Expand Down
24 changes: 0 additions & 24 deletions code/lib/core-server/src/utils/StoryIndexGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1176,30 +1176,6 @@ describe('StoryIndexGenerator', () => {
expect(Object.keys((await generator.getIndex()).entries)).not.toContain('notitle--docs');
});

it('errors on dependency deletion', async () => {
const storiesSpecifier: CoreCommon_NormalizedStoriesSpecifier = normalizeStoriesEntry(
'./src/A.stories.(ts|js|jsx)',
options
);
const docsSpecifier: CoreCommon_NormalizedStoriesSpecifier = normalizeStoriesEntry(
'./src/docs2/*.mdx',
options
);

const generator = new StoryIndexGenerator([docsSpecifier, storiesSpecifier], options);
await generator.initialize();
await generator.getIndex();
expect(toId).toHaveBeenCalledTimes(5);

expect(Object.keys((await generator.getIndex()).entries)).toContain('a--story-one');

generator.invalidate(storiesSpecifier, './src/A.stories.js', true);

await expect(() => generator.getIndex()).rejects.toThrowError(
/Could not find "..\/A.stories" for docs file/
);
});

it('cleans up properly on dependent docs deletion', async () => {
const storiesSpecifier: CoreCommon_NormalizedStoriesSpecifier = normalizeStoriesEntry(
'./src/A.stories.(ts|js|jsx)',
Expand Down
12 changes: 6 additions & 6 deletions code/lib/core-server/src/utils/StoryIndexGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,12 @@ export class StoryIndexGenerator {
dependencies.forEach((dep) => {
if (dep.entries.length > 0) {
const first = dep.entries[0];
if (path.resolve(this.options.workingDir, first.importPath).startsWith(absoluteOf)) {

if (
path
.normalize(path.resolve(this.options.workingDir, first.importPath))
.startsWith(path.normalize(absoluteOf))
) {
ofTitle = first.title;
}
}
Expand Down Expand Up @@ -485,11 +490,6 @@ export class StoryIndexGenerator {
}
});
});

const notFound = dependents.filter((dep) => !invalidated.has(dep));
if (notFound.length > 0) {
throw new Error(`Could not invalidate ${notFound.length} deps: ${notFound.join(', ')}`);
}
}

if (removed) {
Expand Down
14 changes: 7 additions & 7 deletions code/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6469,7 +6469,7 @@ __metadata:
"@storybook/types": 7.0.0-alpha.49
"@types/babel__core": ^7.0.0
"@types/express": ^4.7.0
"@types/mock-fs": ^4.13.0
"@types/mock-fs": ^4.13.1
"@types/node": ^16.0.0
"@types/picomatch": ^2.3.0
"@types/pretty-hrtime": ^1.0.0
Expand All @@ -6483,7 +6483,7 @@ __metadata:
glob: ^7.1.6
handlebars: ^4.7.7
lazy-universal-dotenv: ^3.0.1
mock-fs: ^4.13.0
mock-fs: ^5.2.0
picomatch: ^2.3.0
pkg-dir: ^5.0.0
pretty-hrtime: ^1.0.3
Expand Down Expand Up @@ -8787,7 +8787,7 @@ __metadata:
languageName: node
linkType: hard

"@types/mock-fs@npm:^4.13.0":
"@types/mock-fs@npm:^4.13.1":
version: 4.13.1
resolution: "@types/mock-fs@npm:4.13.1"
dependencies:
Expand Down Expand Up @@ -25893,10 +25893,10 @@ __metadata:
languageName: node
linkType: hard

"mock-fs@npm:^4.13.0":
version: 4.14.0
resolution: "mock-fs@npm:4.14.0"
checksum: a23bc2ce74f2a01d02053fb20aecc2ea359e62580cd15b5e1029b55929802e2770bbd683ccdc5c1eabb5cecbf452196bb81a0ef61c4629dc819023e10d8303c6
"mock-fs@npm:^5.2.0":
version: 5.2.0
resolution: "mock-fs@npm:5.2.0"
checksum: e917e71295ee9d805c7d9ab0214a66658db0212f35731ba0c75dc1ccbb9964e6787a6d725561f05fcf569c64cf4a1176f5ce438f98b009227520f646f8abf174
languageName: node
linkType: hard

Expand Down

0 comments on commit 82dc644

Please sign in to comment.