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.

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

Extend updateConfigAndRun to allow change of verbose and collectCoverage #6467

Closed
tdd opened this issue Jun 14, 2018 · 7 comments · Fixed by #6473
Closed

Extend updateConfigAndRun to allow change of verbose and collectCoverage #6467

tdd opened this issue Jun 14, 2018 · 7 comments · Fixed by #6473

Comments

@tdd
Copy link
Contributor

tdd commented Jun 14, 2018

🚀 Feature Proposal

The current scope of accepted config keys is super narrow (and the documentation doesn't say anything about it).

Motivation

The two most asked-for watch plugins that are not yet implemented are:

  • toggle coverage collection
  • toggle test verbosity

Both are trivial to implement, provided that the passed updateConfigAndRun() function takes the relevant global config keys into account. Currently, it only accepts the minimum set of config keys required to implement the internal plugins, not one more. Fortunately, this was enough to implement the typeahead plugin, as it's just an overwrite of the logic for the non-typeahead internal plugin, but that's as far as it goes.

Note that IMHO, the global config should be audited for prime candidate keys that seem innocuous enough to expose to watch plugins. It's likely that bail, collectCoverageFrom, collectCoverageOnlyFrom, coverageDirectory, coverageReporters, notify, notifyMode, onlyFailures and reporters would be good candidates that do not impede core's ability to function when tweaked.

Example

  • Being able to toggle collectCoverage would let us implement a coverage toggler
  • Being able to toggle verbose would let us implement a verbosity toggler.

Pitch

Why does this feature belong in the Jest core platform?

Because it can't be implemented from the outside. For obvious reasons, the passed global config is immutable, and the one used by watch() is a local reference that it is the only scope to access.

I'm willing to help with changes in core, and have the two aforementioned plugins at the ready (or almost so, for coverage, as I want to expand it further).

Let me know!

@rogeliog
Copy link
Contributor

rogeliog commented Jun 14, 2018

I agree! I think it would be great to expand this to a broader set of config options.

We should also explicitly mention in the docs which are the options that can be overridden. We could also add --coverage.

I'm also happy to review the PR

@tdd
Copy link
Contributor Author

tdd commented Jun 15, 2018

Hey @rogeliog I'm trying to contribute to the codebase, but the current master utterly fails to work for me… Let me elaborate:

  • yarn build works fine
  • yarn lint (used by yarn test) tells me it can't find the eslint-plugin-jest plugin, which doesn't even exist in ESLin't configuration (either in .eslintrc.js or .vscode/settings.json), so I'm not sure why it needs it. This prevents linting.
  • yarn jest fails fast on "Two projects resolved to the same config path", listing the examples/getting_started subdir and Jest's package root dir. Never seen that before.

So I can add code and build, and interactively try it by using npm link or yarn link and playing around with my to-be-published plugins, and I guess I can add to the doc and check the rendering, but I can't lint and run my extra tests.

Any help appreciated 😉

@SimenB
Copy link
Member

SimenB commented Jun 15, 2018

rm -rf examples && git checkout examples && yarn should fix your second issue at least (lingering node_modules after a rename)

@tdd
Copy link
Contributor Author

tdd commented Jun 15, 2018

Hey @SimenB thanks for the tip! It worked like a charm. This was indeed not a clean clone, my fork had existed for a long time and had this leftover node_modules.

@SimenB
Copy link
Member

SimenB commented Jun 15, 2018

the eslint plugin error is probably the same thing - previously that plugin was part of this monorepo. So I'd guess you have a symbolic link in root node_modules pointing to nothing. Just delete it and rerun yarn

@tdd
Copy link
Contributor Author

tdd commented Jun 15, 2018

I went ahead and did a fresh clone of my fork, fixed the whole shenanigans. Thx!

tdd added a commit to deliciousinsights/jest that referenced this issue Jun 22, 2018
SimenB pushed a commit that referenced this issue Jul 12, 2018
* Allow WatchPlugins access to a broader list of global config options

Refs #6467

* Tests for whitelisting config options in WatchPlugins

* Docs on allowed config options in WatchPlugins

* chore: Changelog

* tests: leverage it.each to clean up code

* refactor: centralize config update logic

* Update plugin test to use new signature for configs

* Refactor watch extensions as per PR review (#6473)
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants