Skip to content

Commit

Permalink
Selective test on workspace name with the --package option (#2145)
Browse files Browse the repository at this point in the history
* Add --package option; refactor test logic; add logging

* Create friendly-dancers-fix.md

* Quit when selective tests find no workspaces

* Add tests for package option

* Add unhappy paths test

* Add docs, fix headings

* Add newlines to stderr messages

* Update docs/commands/test.md

Co-authored-by: Sam Brown <sam.brown@jpmorgan.com>

Co-authored-by: Sam Brown <sam.brown@jpmorgan.com>
  • Loading branch information
cristiano-belloni and sgb-io authored Sep 20, 2022
1 parent 1287b1b commit a17e9df
Show file tree
Hide file tree
Showing 6 changed files with 377 additions and 35 deletions.
5 changes: 5 additions & 0 deletions .changeset/friendly-dancers-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"modular-scripts": minor
---

Selective test on workspace name with the --package option
25 changes: 17 additions & 8 deletions docs/commands/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ Specify the comparison branch used to determine which files have changed when
using the `changed` option. If this option is used without `changed`, the
command will fail.

#### collectCoverageFrom
### collectCoverageFrom

[_Documentation_](https://jestjs.io/docs/configuration#collectcoveragefrom-array)

Expand All @@ -88,7 +88,7 @@ Example:
}
```

#### coveragePathIgnorePatterns
### coveragePathIgnorePatterns

[_Documentation_](https://jestjs.io/docs/configuration#coveragepathignorepatterns-arraystring)

Expand All @@ -98,7 +98,7 @@ An array of regexp pattern strings that are matched against all file paths
before executing the test. If the file path matches any of the patterns,
coverage information will be skipped.

#### coverageThreshold
### coverageThreshold

[_Documentation_](https://jestjs.io/docs/configuration#coveragethreshold-object)

Expand All @@ -111,7 +111,7 @@ positive number are taken to be the minimum percentage required. Thresholds
specified as a negative number represent the maximum number of uncovered
entities allowed.

#### moduleNameMapper
### moduleNameMapper

[_Documentation_](https://jestjs.io/docs/configuration#modulenamemapper-objectstring-string--arraystring)

Expand All @@ -131,7 +131,7 @@ The moduleNameMapper is merged with the `modular` defaults to provide common use
cases for static assets, like static assets including images, CSS and
CSS-modules.

#### modulePathIgnorePatterns
### modulePathIgnorePatterns

[_Documentation_](https://jestjs.io/docs/configuration#modulepathignorepatterns-arraystring)

Expand All @@ -142,7 +142,16 @@ before those paths are to be considered 'visible' to the module loader. If a
given module's path matches any of the patterns, it will not be `require()`-able
in the test environment.

#### testPathIgnorePatterns
### package

Default: `undefined`

Run all the tests for the workspace with the specified package name. Can be
repeated to select more than one workspace. Can be combined with the
`--ancestors` option to test the specified workspace(s) plus all the workspaces
that, directly or indirectly, depend on them. Conflicts with `--changed`.

### testPathIgnorePatterns

[_Documentation_](https://jestjs.io/docs/configuration#testpathignorepatterns-arraystring)

Expand All @@ -152,7 +161,7 @@ An array of regexp pattern strings that are matched against all test paths
before executing the test. If the test path matches any of the patterns, it will
be skipped.

#### testRunner
### testRunner

[_Documentation_](https://jestjs.io/docs/configuration#testrunner-string)

Expand All @@ -166,7 +175,7 @@ This can be used to revert to the previous `jest` default testRunner
(`jest-jasmine2`) in cases where `circus` is not yet compatible with an older
codebase.

#### transformIgnorePatterns
### transformIgnorePatterns

[_Documentation_](https://jestjs.io/docs/configuration#transformignorepatterns-arraystring)

Expand Down
203 changes: 203 additions & 0 deletions packages/modular-scripts/src/__tests__/test.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,209 @@ describe('Modular test command', () => {
);
});
});

describe('test command can successfully do selective tests based on selected packages', () => {
const fixturesFolder = path.join(
__dirname,
Array.from({ length: 4 }).reduce<string>(
(acc) => `${acc}..${path.sep}`,
'',
),
'__fixtures__',
'ghost-testing',
);

const currentModularFolder = getModularRoot();
let randomOutputFolder: string;

beforeEach(() => {
// Create random dir
randomOutputFolder = tmp.dirSync({ unsafeCleanup: true }).name;
fs.copySync(fixturesFolder, randomOutputFolder);
execa.sync('yarn', {
cwd: randomOutputFolder,
});
});

// Run in a single test, serially for performance reasons (the setup time is quite long)
it('finds --package after specifying a valid workspaces / finds ancestors using --ancestors', () => {
const resultPackages = runRemoteModularTest(
currentModularFolder,
randomOutputFolder,
['test', '--package', 'b', '--package', 'c'],
);
expect(resultPackages.stderr).toContain(
'packages/c/src/__tests__/utils/c-nested.test.ts',
);
expect(resultPackages.stderr).toContain(
'packages/c/src/__tests__/c.test.ts',
);
expect(resultPackages.stderr).toContain(
'packages/b/src/__tests__/utils/b-nested.test.ts',
);
expect(resultPackages.stderr).toContain(
'packages/b/src/__tests__/b.test.ts',
);

const resultPackagesWithAncestors = runRemoteModularTest(
currentModularFolder,
randomOutputFolder,
['test', '--ancestors', '--package', 'b', '--package', 'c'],
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/c/src/__tests__/utils/c-nested.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/c/src/__tests__/c.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/b/src/__tests__/utils/b-nested.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/b/src/__tests__/b.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/a/src/__tests__/utils/a-nested.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/a/src/__tests__/a.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/e/src/__tests__/utils/e-nested.test.ts',
);
expect(resultPackagesWithAncestors.stderr).toContain(
'packages/e/src/__tests__/e.test.ts',
);
});
});

describe('test command has error states', () => {
// Run in a single test, serially for performance reasons (the setup time is quite long)
it('errors when specifying --package with --changed', async () => {
let errorNumber;
try {
await execa(
'yarnpkg',
['modular', 'test', '--changed', '--package', 'modular-scripts'],
{
all: true,
cleanup: true,
},
);
} catch (error) {
errorNumber = (error as ExecaError).exitCode;
}
expect(errorNumber).toEqual(1);
});

it('errors when specifying --package with a non-existing workspace', async () => {
let capturedError;
try {
await execa(
'yarnpkg',
['modular', 'test', '--package', 'non-existing'],
{
all: true,
cleanup: true,
},
);
} catch (error) {
capturedError = error as ExecaError;
}
expect(capturedError?.exitCode).toEqual(1);
expect(capturedError?.stderr).toContain(
`Package non-existing was specified, but Modular couldn't find it`,
);
});

it('errors when specifying a regex with --packages', async () => {
let capturedError;
try {
await execa(
'yarnpkg',
[
'modular',
'test',
'memoize.test.ts',
'--package',
'modular-scripts',
],
{
all: true,
cleanup: true,
},
);
} catch (error) {
capturedError = error as ExecaError;
}
expect(capturedError?.exitCode).toEqual(1);
expect(capturedError?.stderr).toContain(
`Option --package conflicts with supplied test regex`,
);
});

it('errors when specifying a regex with --package', async () => {
let capturedError;
try {
await execa(
'yarnpkg',
[
'modular',
'test',
'memoize.test.ts',
'--package',
'modular-scripts',
],
{
all: true,
cleanup: true,
},
);
} catch (error) {
capturedError = error as ExecaError;
}
expect(capturedError?.exitCode).toEqual(1);
expect(capturedError?.stderr).toContain(
`Option --package conflicts with supplied test regex`,
);
});

it('errors when specifying a regex with --changed', async () => {
let capturedError;
try {
await execa(
'yarnpkg',
['modular', 'test', 'memoize.test.ts', '--changed'],
{
all: true,
cleanup: true,
},
);
} catch (error) {
capturedError = error as ExecaError;
}
expect(capturedError?.exitCode).toEqual(1);
expect(capturedError?.stderr).toContain(
`Option --changed conflicts with supplied test regex`,
);
});

it('errors when specifying --compareBranch without --changed', async () => {
let capturedError;
try {
await execa('yarnpkg', ['modular', 'test', '--compareBranch', 'main'], {
all: true,
cleanup: true,
});
} catch (error) {
capturedError = error as ExecaError;
}
expect(capturedError?.exitCode).toEqual(1);
expect(capturedError?.stderr).toContain(
`Option --compareBranch doesn't make sense without option --changed`,
);
});
});
});

function runRemoteModularTest(
Expand Down
21 changes: 17 additions & 4 deletions packages/modular-scripts/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ program
'--compareBranch <branch>',
"Specifies the branch to use with the --changed flag. If not specified, Modular will use the repo's default branch",
)
.option('--package <packages...>', 'Specifies one or more packages to test')
.option('--coverage', testOptions.coverage.description)
.option('--forceExit', testOptions.forceExit.description)
.option('--env <env>', testOptions.env.description, 'jsdom')
Expand All @@ -171,21 +172,33 @@ program
.allowUnknownOption()
.description('Run tests over the codebase')
.action(async (regexes: string[], options: CLITestOptions) => {
if (options.ancestors && !options.changed) {
if (options.ancestors && !options.changed && !options.package) {
process.stderr.write(
"Option --ancestors doesn't make sense without option --changed",
"Option --ancestors doesn't make sense without option --changed or option --package\n",
);
process.exit(1);
}
if (options.package && options.changed) {
process.stderr.write(
'Option --package conflicts with option --changed\n',
);
process.exit(1);
}
if (options.compareBranch && !options.changed) {
process.stderr.write(
"Option --compareBranch doesn't make sense without option --changed",
"Option --compareBranch doesn't make sense without option --changed\n",
);
process.exit(1);
}
if (options.changed && regexes.length) {
process.stderr.write(
'Option --changed conflicts with supplied test regex',
'Option --changed conflicts with supplied test regex\n',
);
process.exit(1);
}
if (options.package && regexes.length) {
process.stderr.write(
'Option --package conflicts with supplied test regex\n',
);
process.exit(1);
}
Expand Down
Loading

0 comments on commit a17e9df

Please sign in to comment.