-
-
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
feat(jest-config)!: remove undocumented collectCoverageOnlyFrom
option
#13156
feat(jest-config)!: remove undocumented collectCoverageOnlyFrom
option
#13156
Conversation
@@ -213,7 +213,7 @@ final class JestUnitTestEngine extends ArcanistUnitTestEngine { | |||
if (count($paths) < self::TOO_MANY_FILES_TO_COVER) { | |||
$options[] = '--findRelatedTests ' . join(' ', $paths); | |||
$options[] = '--coverage'; | |||
$options[] = '--collectCoverageOnlyFrom '. join(' ', $paths); | |||
$options[] = '--collectCoverageFrom '. join(' ', $paths); |
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.
Hm.. Not sure how to try this out.
@@ -157,7 +157,6 @@ For stability and safety reasons, only part of the global configuration keys can | |||
- [`changedSince`](cli#--changedsince) | |||
- [`collectCoverage`](configuration#collectcoverage-boolean) | |||
- [`collectCoverageFrom`](configuration#collectcoveragefrom-array) | |||
- [`collectCoverageOnlyFrom`](configuration#collectcoverageonlyfrom-array) |
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.
collectCoverageOnlyFrom
was not documented, so this link was broken in versioned docs.
describe('collectCoverageFrom', () => { | ||
it('substitutes <rootDir> tokens', async () => { | ||
it('ignores <rootDir> tokens', async () => { |
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.
Could not leave it. In this case the tokens are ignored. See the test or code here: https://github.com/facebook/jest/blob/3bef02e5925948b38f035572121a41ec9b633dfa/packages/jest-config/src/normalize.ts#L301-L305
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.
works for me (with the added deprecation warning), thanks!
Exciting! I never got back to it, would love to see it more complete 👍 |
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Just thinking out loud. The documentation for
collectCoverageOnlyFrom
option was removed in #1724. Looking at #1349 (comment) and #1536 (comment) it feels like there was a plan to remove the option as well.Nothing against it (; Simply I am playing with JSON schema for Jest config. Each extra option is addition trouble, because it needs tests, useful description, etc.
Perhaps it is time to remove
collectCoverageOnlyFrom
?Test plan
Green CI.