Skip to content

Feature: broader uses for watch plugins #6473

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

Merged
SimenB merged 11 commits into from
Jul 12, 2018
Merged

Feature: broader uses for watch plugins #6473

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
f21640d
Allow WatchPlugins access to a broader list of global config options
tdd Jun 15, 2018
21a93b4
Tests for whitelisting config options in WatchPlugins
tdd Jun 15, 2018
bd8a325
Docs on allowed config options in WatchPlugins
tdd Jun 15, 2018
cfa476e
chore: Changelog
tdd Jun 15, 2018
e09f018
tests: leverage it.each to clean up code
tdd Jun 20, 2018
a5f5c4b
refactor: centralize config update logic
tdd Jun 20, 2018
1cd159a
Merge branch 'master' into feat/broader-watch-plugins
tdd Jul 7, 2018
1366412
Merge branch 'master' into feat/broader-watch-plugins
rogeliog Jul 8, 2018
e093885
Update plugin test to use new signature for configs
rogeliog Jul 8, 2018
8793918
Refactor watch extensions as per PR review (#6473)
tdd Jul 10, 2018
f36e644
Merge branch 'master' into feat/broader-watch-plugins
tdd Jul 11, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Docs on allowed config options in WatchPlugins
  • Loading branch information
@tdd
tdd committed Jun 22, 2018
commit bd8a325b4c3a77ca094235ce9f080efde202bf6f
21 changes: 21 additions & 0 deletions docs/WatchPlugins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,3 +148,24 @@ class MyWatchPlugin {
}
}
```

**Note**: If you do call `updateConfigAndRun`, your `run` method should not resolve to a truthy value, as that would trigger a double-run.

#### Authorized configuration keys

For stability and safety reasons, only part of the global configuration keys can be updated with `updateConfigAndRun`. The current white list is as follows:
Copy link
Member

@SimenB SimenB Jun 15, 2018

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?

Copy link
Contributor Author

@tdd tdd Jun 15, 2018

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 by getChangedFilesForRoots in jest-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 the a 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?

Copy link
Member

@SimenB SimenB Jun 15, 2018

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 think changedSince would be useful. It would then overwrite onlyChanged.

Copy link
Contributor Author

@tdd tdd Jun 15, 2018

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 or lastCommit, 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.

Copy link
Member

@SimenB SimenB Jun 16, 2018

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 😀

Copy link
Contributor

@rogeliog rogeliog Jun 19, 2018

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 🙃


- [`bail`](configuration.html#bail-boolean)
- [`collectCoverage`](configuration.html#collectcoverage-boolean)
- [`collectCoverageFrom`](configuration.html#collectcoveragefrom-array)
- [`collectCoverageOnlyFrom`](configuration.html#collectcoverageonlyfrom-array)
- [`coverageDirectory`](configuration.html#coveragedirectory-string)
- [`coverageReporters`](configuration.html#coveragereporters-array)
- [`notify`](configuration.html#notify-boolean)
- [`notifyMode`](configuration.html#notifymode-string)
- [`onlyFailures`](configuration.html#onlyfailures-boolean)
- [`reporters`](configuration.html#reporters-array-modulename-modulename-options)
- [`testNamePattern`](cli.html#testnamepattern-regex)
- [`testPathPattern`](cli.html#testpathpattern-regex)
- [`updateSnapshot`](cli.html#updatesnapshot)
- [`verbose`](configuration.html#verbose-boolean)