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

Add meta-data arg to JSON event #5208

Merged
merged 1 commit into from
Jan 3, 2018
Merged

Add meta-data arg to JSON event #5208

merged 1 commit into from
Jan 3, 2018

Conversation

seanpoulter
Copy link
Contributor

@seanpoulter seanpoulter commented Jan 2, 2018

Summary
When running Jest in watch mode, the test results that follow the "No tests found related to files changed since last commit" message will be empty. This PR adds detection for this message, and emits this info with the JSON test results. This behavior is required for jest-community/vscode-jest#213.

Test plan
Tests have been added, and I've confirmed the expected behavior debugging vscode-jest.

/cc @orta

When running Jest in watch mode, the test results that follow the "No
tests found related to files changed since last commit" message will be
empty. This commit adds detection for this message, and emits this info
with the JSON test results.
@codecov-io
Copy link

Codecov Report

Merging #5208 into master will increase coverage by 0.1%.
The diff coverage is 52.38%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #5208     +/-   ##
========================================
+ Coverage    60.8%   60.9%   +0.1%     
========================================
  Files         201     201             
  Lines        6707    6725     +18     
  Branches        3       4      +1     
========================================
+ Hits         4078    4096     +18     
  Misses       2628    2628             
  Partials        1       1
Impacted Files Coverage Δ
packages/jest-editor-support/src/Runner.js 63.38% <52.38%> (+12.43%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8549900...baed45e. Read the comment docs.

@orta
Copy link
Member

orta commented Jan 3, 2018

Yep, this looks good to me - thanks @seanpoulter

@orta orta merged commit 21b608c into jestjs:master Jan 3, 2018
@cpojer
Copy link
Member

cpojer commented Jan 3, 2018

I think it would be great to have some integration tests for this stuff. Right now, when we change text in watch mode, we’d just break this unless people remember to search for usage of these strings.

@seanpoulter
Copy link
Contributor Author

Good call @cpojer. Are there any similar integration tests running Jest I can look at for inspiration?

@cpojer
Copy link
Member

cpojer commented Jan 3, 2018

Unfortunately we don't have anything for this yet.

@orta
Copy link
Member

orta commented Jan 3, 2018

Could the test pull out the CLI source code and verify that the flags are still there?

E.g. run check with --watch and validate that it gets the watchMode set? - https://github.com/facebook/jest/blob/master/packages/jest-cli/src/cli/args.js x

@seanpoulter
Copy link
Contributor Author

seanpoulter commented Jan 19, 2018

Sorry for not keeping this one alive. 😞 I'll build on your idea @orta and figure out if we can print the expected strings without the fancy colors.

Edit: Actually, a full integration test spawning Jest is quicker than I expected and is being pretty informative. I'll build it out for the "Watch Usage" prompt and send another PR.

@github-actions
Copy link

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.
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 this pull request may close these issues.

5 participants