-
-
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
Allow unmatched test paths from CLI #3882
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3882 +/- ##
==========================================
- Coverage 60.58% 60.56% -0.03%
==========================================
Files 201 201
Lines 6686 6690 +4
Branches 4 4
==========================================
+ Hits 4051 4052 +1
- Misses 2635 2638 +3
Continue to review full report at Codecov.
|
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.
aren't those two separate issues?
i'd split it into two PRs
the console
part looks awesome :)
@@ -76,6 +77,15 @@ const toTests = (context, tests) => | |||
path, | |||
})); | |||
|
|||
const fileExists = (filePath: string) => { |
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.
we already have like 3 functions like this in the codebase. It sucks that we can't just share it :(
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 would share it between packages too, but where to put it? Into jest-util
?
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.
just use fs.existsSync()
)
@@ -57,25 +59,28 @@ class DefaultReporter extends BaseReporter { | |||
let buffer = []; | |||
let timeout = null; | |||
|
|||
const doFlush = () => { | |||
const flushBufferedIO = () => { |
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.
it's not really an IO
, more like.. just O
:)
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.
maybe just call it flushBufferedOutput
?
return true; | ||
}; | ||
} | ||
|
||
flushDebouncedIO() { | ||
for (const io of this._bufferedIO) { | ||
io(); |
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.
this should be flushBufferedOutput()
as well
463c7e8
to
8137ae2
Compare
Removed part about |
cc @aaronabramov what's the status of this? Is this good for 21? |
I'd like to see a flag added for the old behaviour. Or at least output a string saying that during a normal test run the file would not be executed. It's useful info when trying to understand why a test is not run in CI (ref #4630) |
/** | ||
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved. | ||
* | ||
* This source code is licensed under the BSD-style license found in the |
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.
should be MIT
@valerybugakov do you mind addressing the change requests by @SimenB? |
2b8db4a
to
2a360b2
Compare
@cpojer np, rebased with master |
@valerybugakov do you mind also fixing the tests so we can land this? :) |
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
Fixes #3755 and #3793
Test plan
Examples provided in issues above.
Added an ability to run tests by provided filenames:
jest ../foo.js ../tests/bar.js ./src/baz.js
Will add tests if fixes are ok.