-
-
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: add jest-watch package #6318
Conversation
@@ -13,7 +13,8 @@ import type {AggregatedResult, AssertionLocation} from 'types/TestResult'; | |||
const chalk = require('chalk'); | |||
const ansiEscapes = require('ansi-escapes'); | |||
const {pluralize} = require('./reporters/utils'); | |||
const {KEYS, ARROW} = require('./constants'); | |||
const {ARROW} = require('./constants'); | |||
import {KEYS} from 'jest-watch'; |
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.
Move import
above const
s
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.
wait, why are these require
s? Didn't even notice
"dependencies": { | ||
"ansi-escapes": "^3.0.0", | ||
"chalk": "^2.0.1", | ||
"string-length": "^2.0.0" |
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.
Please make sure these are all deps jest-watch
use
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.
I copied jest-cli
and removed unused stuff, so it should be enough
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.
haven't checked if I can remove anything from jest-cli, though, lemme do that
EDIT: All 3 still used in cli
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.
Nice!
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.
Looks good, I'm just curious about what the responsibilities of jest-watch
are going to be?
- Should this eventually evolve into a standalone "watch" package that can watch a filesystem and execute stuff against it and react to fs events? Or should it mainly be a place to make the initial plugins easier to develop?
This I think. So maybe the name is too generic? We already have |
Codecov Report
@@ Coverage Diff @@
## master #6318 +/- ##
==========================================
+ Coverage 63.65% 63.87% +0.22%
==========================================
Files 227 228 +1
Lines 8637 8698 +61
Branches 3 3
==========================================
+ Hits 5498 5556 +58
- Misses 3138 3141 +3
Partials 1 1
Continue to review full report at Codecov.
|
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
This splits out the parts needed to make watch plugins out of
jest-cli
into a new package calledjest-watch
. I'm quite unfamiliar with watch mode, so I might have missed something, or included something which doesn't have to moveTest plan
All existing tests should pass.