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.

Sign up for GitHub

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

Jump to bottom

test: allow more tests to run on windows #10351

Merged
merged 4 commits into from Aug 2, 2020
Merged

test: allow more tests to run on windows #10351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b3754b5
test: allow tests to run on windows
G-Rath Aug 1, 2020
3cd6fa4
test: remove `skipSuiteOnWindows` helper
G-Rath Aug 1, 2020
88cfb17
tmp: fix haste-map tests more
G-Rath Aug 2, 2020
9f8b310
test: escape Windows path separators in circus mock
G-Rath Aug 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 3 additions & 4 deletions e2e/__tests__/cliHandlesExactFilenames.test.ts
View file
Open in desktop
Expand Up @@ -7,14 +7,11 @@

import * as path from 'path';
import {wrap} from 'jest-snapshot-serializer-raw';
import {skipSuiteOnWindows} from '@jest/test-utils';
import {cleanup, extractSummary, writeFiles} from '../Utils';
import runJest from '../runJest';

const DIR = path.resolve(__dirname, '../cli_accepts_exact_filenames');

skipSuiteOnWindows();

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

Expand All @@ -30,7 +27,9 @@ test('CLI accepts exact file names if matchers matched', () => {

expect(result.exitCode).toBe(0);

const {rest, summary} = extractSummary(result.stderr);
const {rest, summary} = extractSummary(
result.stderr.replace('\\\\foo\\\\bar', '\\/foo\\/bar'),
Copy link
Member

Choose a reason for hiding this comment

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

Could we normalize the output of jest instead of normalizing in the test itself?

Copy link
Contributor Author

@G-Rath G-Rath Aug 2, 2020 •
edited

Choose a reason for hiding this comment

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

Technically, yes but I don't know how much work that'll involve.

However, I don't know if thats a good idea from UX POV, as jest is implicitly normalizing the input - we pass in ./foo/bar.spec.js, which is a Linux style file path (it's actually probably node doing the normalizing, but the result is the same).

By outputting the actual OS-specific file path that was used, users can see that it doesn't matter if they pass in Linux file paths; where as if we normalised them in the output here, personally I'd look at that and think "oh maybe Jest isn't finding my test file because it's using the Linux file path" (especially since copying & pasting the file path into Windows would cause an error, as iirc Powershell, CMD, and Explorer all expect forward slashes).

);

expect(wrap(rest)).toMatchSnapshot();
expect(wrap(summary)).toMatchSnapshot();
Expand Down
9 changes: 3 additions & 6 deletions e2e/__tests__/dependencyClash.test.ts
View file
Open in desktop
Expand Up @@ -7,17 +7,14 @@

import * as path from 'path';
import {tmpdir} from 'os';
import {skipSuiteOnWindows} from '@jest/test-utils';
import {cleanup, createEmptyPackage, writeFiles} from '../Utils';
import runJest from '../runJest';

skipSuiteOnWindows();

// doing test in a temp directory because we don't want jest node_modules affect it
const tempDir = path.resolve(tmpdir(), 'clashing-dependencies-test');
const hasteImplModulePath = path.resolve(
G-Rath marked this conversation as resolved.
Show resolved Hide resolved
'./packages/jest-haste-map/src/__tests__/haste_impl.js',
);
const hasteImplModulePath = path
.resolve('./packages/jest-haste-map/src/__tests__/haste_impl.js')
.replace(/\\/g, '\\\\');

beforeEach(() => {
cleanup(tempDir);
Expand Down
10 changes: 7 additions & 3 deletions e2e/__tests__/detectOpenHandles.ts
View file
Open in desktop
Expand Up @@ -6,7 +6,7 @@
*/

import {wrap} from 'jest-snapshot-serializer-raw';
import {onNodeVersions, skipSuiteOnWindows} from '@jest/test-utils';
import {onNodeVersions} from '@jest/test-utils';
import runJest, {runContinuous} from '../runJest';

try {
Expand Down Expand Up @@ -72,9 +72,13 @@ it('does not report promises', () => {
});

describe('notify', () => {
skipSuiteOnWindows();

it('does not report --notify flag', () => {
if (process.platform === 'win32') {
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same as the skip?

Copy link
Contributor Author

@G-Rath G-Rath Aug 1, 2020 •
edited

Choose a reason for hiding this comment

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

No, as the skip causes all tests in the file to be skipped, but this is the only one that actually fails.

Copy link
Contributor Author

@G-Rath G-Rath Aug 1, 2020 •
edited

Choose a reason for hiding this comment

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

This is the only test aside from the yarn pnp one that is now being skipped on Windows 🎉

Open to ideas on if anything can be done to eliminate it - I suspect it represents an actual "bug" (/ maybe just something that can't be supported?)

console.warn('[SKIP] Does not work on Windows');

return;
}

// The test here is basically that it exits cleanly without reporting anything (does not need `until`)
const {stderr} = runJest('detect-open-handles', ['notify', '--notify']);
const textAfterTest = getTextAfterTest(stderr);
Expand Down
10 changes: 6 additions & 4 deletions e2e/__tests__/existentRoots.test.ts
View file
Open in desktop
Expand Up @@ -7,7 +7,7 @@

import * as path from 'path';
import {tmpdir} from 'os';
import {skipSuiteOnWindows} from '@jest/test-utils';
import {tryRealpath} from 'jest-util';
import {cleanup, writeFiles} from '../Utils';
import runJest from '../runJest';

Expand All @@ -16,8 +16,6 @@ const DIR = path.resolve(tmpdir(), 'existent-roots');
beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

skipSuiteOnWindows();

function writeConfig(rootDir: string, roots?: Array<string>) {
writeFiles(DIR, {
'jest.config.js': `
Expand All @@ -40,7 +38,11 @@ test('error when rootDir does not exist', () => {
});

test('error when rootDir is a file', () => {
const fakeRootDir = path.join(DIR, 'jest.config.js');
// Replace tmpdir with its realpath as Windows uses the 8.3 path
const fakeRootDir = path
.join(DIR, 'jest.config.js')
.replace(tmpdir(), tryRealpath(tmpdir()));

writeConfig(fakeRootDir);

const {exitCode, stderr} = runJest(DIR);
Expand Down
28 changes: 19 additions & 9 deletions e2e/__tests__/jestChangedFiles.test.ts
View file
Open in desktop
Expand Up @@ -9,12 +9,10 @@ import {tmpdir} from 'os';
import * as path from 'path';
import {wrap} from 'jest-snapshot-serializer-raw';
import {findRepos, getChangedFilesForRoots} from 'jest-changed-files';
import {skipSuiteOnWindows} from '@jest/test-utils';
import slash from 'slash';
import {cleanup, run, testIfHg, writeFiles} from '../Utils';
import runJest from '../runJest';

skipSuiteOnWindows();

const DIR = path.resolve(tmpdir(), 'jest-changed-files-test-dir');

const GIT = 'git -c user.name=jest_test -c user.email=jest_test@test.com';
Expand Down Expand Up @@ -54,8 +52,12 @@ testIfHg('gets hg SCM roots and dedupes them', async () => {
// NOTE: This test can break if you have a .hg repo initialized inside your
// os tmp directory.
expect(hgRepos).toHaveLength(2);
expect(hgRepos[0]).toMatch(/\/jest-changed-files-test-dir\/first-repo\/?$/);
expect(hgRepos[1]).toMatch(/\/jest-changed-files-test-dir\/second-repo\/?$/);
expect(slash(hgRepos[0])).toMatch(
/\/jest-changed-files-test-dir\/first-repo\/?$/,
);
expect(slash(hgRepos[1])).toMatch(
/\/jest-changed-files-test-dir\/second-repo\/?$/,
);
});

test('gets git SCM roots and dedupes them', async () => {
Expand Down Expand Up @@ -88,8 +90,12 @@ test('gets git SCM roots and dedupes them', async () => {
// NOTE: This test can break if you have a .git repo initialized inside your
// os tmp directory.
expect(gitRepos).toHaveLength(2);
expect(gitRepos[0]).toMatch(/\/jest-changed-files-test-dir\/first-repo\/?$/);
expect(gitRepos[1]).toMatch(/\/jest-changed-files-test-dir\/second-repo\/?$/);
expect(slash(gitRepos[0])).toMatch(
/\/jest-changed-files-test-dir\/first-repo\/?$/,
);
expect(slash(gitRepos[1])).toMatch(
/\/jest-changed-files-test-dir\/second-repo\/?$/,
);
});

testIfHg('gets mixed git and hg SCM roots and dedupes them', async () => {
Expand Down Expand Up @@ -121,8 +127,12 @@ testIfHg('gets mixed git and hg SCM roots and dedupes them', async () => {
// inside your os tmp directory.
expect(gitRepos).toHaveLength(1);
expect(hgRepos).toHaveLength(1);
expect(gitRepos[0]).toMatch(/\/jest-changed-files-test-dir\/first-repo\/?$/);
expect(hgRepos[0]).toMatch(/\/jest-changed-files-test-dir\/second-repo\/?$/);
expect(slash(gitRepos[0])).toMatch(
/\/jest-changed-files-test-dir\/first-repo\/?$/,
);
expect(slash(hgRepos[0])).toMatch(
/\/jest-changed-files-test-dir\/second-repo\/?$/,
);
});

test('gets changed files for git', async () => {
Expand Down
11 changes: 7 additions & 4 deletions e2e/__tests__/pnp.test.ts
View file
Open in desktop
Expand Up @@ -6,20 +6,23 @@
*/

import * as path from 'path';
import {skipSuiteOnWindows} from '@jest/test-utils';
import {json as runWithJson} from '../runJest';
import {runYarn} from '../Utils';

const DIR = path.resolve(__dirname, '..', 'pnp');

// https://github.com/facebook/jest/pull/8094#issuecomment-471220694
skipSuiteOnWindows();

beforeEach(() => {
runYarn(DIR, {YARN_NODE_LINKER: 'pnp'});
});

it('successfully runs the tests inside `pnp/`', () => {
// https://github.com/facebook/jest/pull/8094#issuecomment-471220694
if (process.platform === 'win32') {
G-Rath marked this conversation as resolved.
Show resolved Hide resolved
console.warn('[SKIP] Does not work on Windows');

return;
}

const {json} = runWithJson(DIR, ['--no-cache', '--coverage'], {
env: {YARN_NODE_LINKER: 'pnp'},
nodeOptions: `--require ${DIR}/.pnp.js`,
Expand Down
5 changes: 2 additions & 3 deletions e2e/__tests__/showConfig.test.ts
View file
Open in desktop
Expand Up @@ -8,12 +8,9 @@
import * as path from 'path';
import {tmpdir} from 'os';
import {wrap} from 'jest-snapshot-serializer-raw';
import {skipSuiteOnWindows} from '@jest/test-utils';
import runJest from '../runJest';
import {cleanup, writeFiles} from '../Utils';

skipSuiteOnWindows();

const DIR = path.resolve(tmpdir(), 'show-config-test');

beforeEach(() => cleanup(DIR));
Expand All @@ -33,6 +30,8 @@ test('--showConfig outputs config info and exits', () => {
]);

stdout = stdout
.replace(/\\\\node_modules\\\\/g, 'node_modules')
.replace(/\\\\(?:([^.]+?)|$)/g, '/$1')
.replace(/"cacheDirectory": "(.+)"/g, '"cacheDirectory": "/tmp/jest"')
.replace(/"name": "(.+)"/g, '"name": "[md5 hash]"')
.replace(/"version": "(.+)"/g, '"version": "[version]"')
Expand Down
21 changes: 13 additions & 8 deletions packages/jest-circus/src/__mocks__/testUtils.ts
View file
Open in desktop
Expand Up @@ -11,15 +11,20 @@ import {createHash} from 'crypto';
import * as fs from 'graceful-fs';
// eslint-disable-next-line import/named
import {ExecaSyncReturnValue, sync as spawnSync} from 'execa';
import {skipSuiteOnWindows} from '@jest/test-utils';

const CIRCUS_PATH = require.resolve('../../build');
const CIRCUS_RUN_PATH = require.resolve('../../build/run');
const CIRCUS_STATE_PATH = require.resolve('../../build/state');
const TEST_EVENT_HANDLER_PATH = require.resolve('./testEventHandler');
const BABEL_REGISTER_PATH = require.resolve('@babel/register');

skipSuiteOnWindows();
const CIRCUS_PATH = require.resolve('../../build').replace(/\\/g, '\\\\');
const CIRCUS_RUN_PATH = require
.resolve('../../build/run')
.replace(/\\/g, '\\\\');
const CIRCUS_STATE_PATH = require
.resolve('../../build/state')
.replace(/\\/g, '\\\\');
const TEST_EVENT_HANDLER_PATH = require
.resolve('./testEventHandler')
.replace(/\\/g, '\\\\');
const BABEL_REGISTER_PATH = require
.resolve('@babel/register')
.replace(/\\/g, '\\\\');

interface Result extends ExecaSyncReturnValue {
status: number;
Expand Down