-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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.
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
Feature: broader uses for watch plugins #6473
Changes from 10 commits
f21640d
21a93b4
bd8a325
cfa476e
e09f018
a5f5c4b
1cd159a
1366412
e093885
8793918
f36e644
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -86,6 +86,11 @@ jest.doMock( | |
{virtual: true}, | ||
); | ||
|
||
const regularUpdateGlobalConfig = require('../lib/update_global_config') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but as I'm mocking it right the next line, it seemed more interesting to keep it close by? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
.default; | ||
const updateGlobalConfig = jest.fn(regularUpdateGlobalConfig); | ||
jest.doMock('../lib/update_global_config', () => updateGlobalConfig); | ||
|
||
const watch = require('../watch').default; | ||
|
||
const nextTick = () => new Promise(res => process.nextTick(res)); | ||
|
@@ -435,6 +440,101 @@ describe('Watch mode flows', () => { | |
}); | ||
}); | ||
|
||
it.each` | ||
ok | option | ||
✔︎ | bail | ||
✖︎ | changedFilesWithAncestor | ||
✖︎ | changedSince | ||
✔︎ | collectCoverage | ||
✔︎ | collectCoverageFrom | ||
✔︎ | collectCoverageOnlyFrom | ||
✔︎ | coverageDirectory | ||
✔︎ | coverageReporters | ||
✖︎ | coverageThreshold | ||
✖︎ | detectLeaks | ||
✖︎ | detectOpenHandles | ||
✖︎ | enabledTestsMap | ||
✖︎ | errorOnDeprecated | ||
✖︎ | expand | ||
✖︎ | filter | ||
✖︎ | findRelatedTests | ||
✖︎ | forceExit | ||
✖︎ | globalSetup | ||
✖︎ | globalTeardown | ||
✖︎ | json | ||
✖︎ | lastCommit | ||
✖︎ | listTests | ||
✖︎ | logHeapUsage | ||
✖︎ | maxWorkers | ||
✖︎ | nonFlagArgs | ||
✖︎ | noSCM | ||
✖︎ | noStackTrace | ||
✔︎ | notify | ||
✔︎ | notifyMode | ||
✖︎ | onlyChanged | ||
✔︎ | onlyFailures | ||
✖︎ | outputFile | ||
✖︎ | passWithNoTests | ||
✖︎ | projects | ||
✖︎ | replname | ||
✔︎ | reporters | ||
✖︎ | rootDir | ||
✖︎ | runTestsByPath | ||
✖︎ | silent | ||
✖︎ | skipFilter | ||
✖︎ | testFailureExitCode | ||
✔︎ | testNamePattern | ||
✔︎ | testPathPattern | ||
✖︎ | testResultsProcessor | ||
✔︎ | updateSnapshot | ||
✖︎ | useStderr | ||
✔︎ | verbose | ||
✖︎ | watch | ||
✖︎ | watchAll | ||
✖︎ | watchman | ||
✖︎ | watchPlugins | ||
`( | ||
'allows WatchPlugins to modify only white-listed global config keys', | ||
async ({ok, option}) => { | ||
const pluginPath = `${__dirname}/__fixtures__/plugin_path_config_updater`; | ||
const config = Object.assign({}, globalConfig, { | ||
rootDir: __dirname, | ||
watchPlugins: [{config: {}, path: pluginPath}], | ||
}); | ||
|
||
jest.doMock( | ||
pluginPath, | ||
() => | ||
class WatchPlugin { | ||
getUsageInfo() { | ||
return {key: 'x', prompt: 'test option white-listing'}; | ||
} | ||
|
||
run(globalConfig, updateConfigAndRun) { | ||
updateConfigAndRun({[option]: '__JUST_TRYING__'}); | ||
return Promise.resolve(); | ||
} | ||
}, | ||
{virtual: true}, | ||
); | ||
|
||
watch(config, contexts, pipe, hasteMapInstances, stdin); | ||
await nextTick(); | ||
|
||
stdin.emit('x'); | ||
await nextTick(); | ||
|
||
const lastCall = updateGlobalConfig.mock.calls.slice(-1)[0]; | ||
let expector = expect(lastCall[0]); | ||
if (!ok) { | ||
expector = expector.not; | ||
} | ||
expector.toMatchObject({ | ||
[option]: '__JUST_TRYING__', | ||
}); | ||
}, | ||
); | ||
|
||
it('triggers enter on a WatchPlugin when its key is pressed', async () => { | ||
const run = jest.fn(() => Promise.resolve()); | ||
const pluginPath = `${__dirname}/__fixtures__/plugin_path`; | ||
|
@@ -588,7 +688,6 @@ describe('Watch mode flows', () => { | |
|
||
expect(runJestMock.mock.calls[0][0].globalConfig).toMatchObject({ | ||
testNamePattern: 'test', | ||
testPathPattern: '', | ||
watch: true, | ||
watchAll: false, | ||
}); | ||
|
@@ -606,7 +705,6 @@ describe('Watch mode flows', () => { | |
await nextTick(); | ||
|
||
expect(runJestMock.mock.calls[0][0].globalConfig).toMatchObject({ | ||
testNamePattern: '', | ||
testPathPattern: 'file', | ||
watch: true, | ||
watchAll: false, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://facebook.github.io/jest/docs/en/cli.html#changedsince, https://facebook.github.io/jest/docs/en/cli.html#lastcommit and https://facebook.github.io/jest/docs/en/cli.html#onlychanged would be awesome as well. Maybe https://facebook.github.io/jest/docs/en/cli.html#logheapusage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @SimenB!
--changedSince
and--lastCommit
do not have any direct equivalent in the global configuration object. They're used bygetChangedFilesForRoots
injest-changed-files
, and are also ignored when--onlyChanged
(which is the default watch mode), so they don't seem to make sense in that mode. At any rate, the watch architecture would not let us change that on the fly right now, this is a bootstrap-time computation.--onlyChanged
is already togglable in watch mode: these are thea
key (all tests, same as--watchAll
) vs.o
key (only changed tests, same as--onlyChanged
).--logHeapUsage
does have a matching boolean in the global config, so I'm open to adding it, what do others think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in the same way toggling
onlyChanged
is useful, I thinkchangedSince
would be useful. It would then overwriteonlyChanged
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimenB I agree it would be nice, but Jest's current architecture doesn't let us do it. It computes the watched list at boot time, then keeps it in a closure where specific global config options may amend it (such as the watch mode). There's no stuff in there right now for
changedSince
orlastCommit
, and I feel the change would be significant.This specific PR is bound by what's in the global config. Should this feature set become config-based, I'll be glad to expand the white list, but it's a different feature scope IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rogeliog sounds like we need another of your magical refactor runs 😀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we would need to change a couple of things in order to make that happen 🙃