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

Fix testNamePattern value with interactive snapshots #6579

Merged
merged 3 commits into from
Jul 3, 2018

Conversation

wyze
Copy link
Contributor

@wyze wyze commented Jun 29, 2018

Summary

This fixes a regression with interactive snapshot mode where it would fail to find any tests to run in the file. Problem here is it was using the test title with regex pattern to say match the title exactly. But testNamePattern also includes any describe blocks in the title, so we need to include that as well.

I switched from using the test title to fullName and it appears to be working as expected now. Since @rickhanlonii, you did the improvements recently, I'd love some feedback from you on this and if it is the correct way to solve it. Also if there is some end to end test that could be done to ensure this is working as intended.

This fix should resolve #6485 and #6431.

Test plan

All tests pass.

@SimenB
Copy link
Member

SimenB commented Jun 30, 2018

Thanks for the PR! Would be awesome with a test for the regression as well

@wyze
Copy link
Contributor Author

wyze commented Jun 30, 2018

@SimenB Mind pointing me in the right direction on where I can add that? I looked at e2e tests, but not sure how to get those tests to accept input. I've tried adding a test to watch in jest-cli, but not sure it is working like I would expect.

@rickhanlonii
Copy link
Member

Ah, yes great fix. I tested this on the repro repo in #6485 and confirmed that it works 👍

afaik we don't have any integration tests for watch mode, and this is just s/result.title/result.fullName so this is probably OK to go without a regression test

@rickhanlonii
Copy link
Member

Merged in master which should fix the node 6 issue

@SimenB
Copy link
Member

SimenB commented Jul 1, 2018

We might wanna set up some e2e tests for watch mode. Should be easy enough to feed some keypresses into stdin. This PR should not be blocked by it, though

@SimenB SimenB merged commit 4ec046b into jestjs:master Jul 3, 2018
@wyze wyze deleted the fix-interactive-snapshot-testpathname branch July 3, 2018 17:38
@simonrepp
Copy link

Thanks to everyone for your effort! 🎈

schalkneethling referenced this pull request in mdn/interactive-examples Jul 6, 2018
This Pull Request renovates the package group "jest monorepo".


-   [jest-environment-node](https://github.com/facebook/jest) (`devDependencies`): from `23.2.0` to `23.3.0`
-   [jest](https://github.com/facebook/jest) (`devDependencies`): from `23.2.0` to `23.3.0`

# Release Notes
<details>
<summary>facebook/jest</summary>

### [`v23.3.0`](https://github.com/facebook/jest/blob/master/CHANGELOG.md#&#8203;2330)
[Compare Source](jestjs/jest@v23.2.0...v23.3.0)
##### Features

- `[jest-cli]` Allow watch plugin to be configured ([#&#8203;6603](`https://github.com/facebook/jest/pull/6603`))
- `[jest-snapshot]` Introduce `toMatchInlineSnapshot` and `toThrowErrorMatchingInlineSnapshot` matchers ([#&#8203;6380](`https://github.com/facebook/jest/pull/6380`))
##### Fixes

- `[jest-regex-util]` Improve handling already escaped path separators on Windows ([#&#8203;6523](`https://github.com/facebook/jest/pull/6523`))
- `[jest-cli]` Fix `testNamePattern` value with interactive snapshots ([#&#8203;6579](`https://github.com/facebook/jest/pull/6579`))
- `[jest-cli]` Fix enter to interrupt watch mode ([#&#8203;6601](`https://github.com/facebook/jest/pull/6601`))
##### Chore & Maintenance

- `[website]` Switch domain to https://jestjs.io ([#&#8203;6549](`https://github.com/facebook/jest/pull/6549`))
- `[tests]` Improve stability of `yarn test` on Windows ([#&#8203;6534](`https://github.com/facebook/jest/pull/6534`))
- `[*]` Transpile object shorthand into Node 4 compatible syntax ([#&#8203;6582](`https://github.com/facebook/jest/pull/6582`))
- `[*]` Update all legacy links to jestjs.io ([#&#8203;6622](`https://github.com/facebook/jest/pull/6622`))
- `[docs]` Add docs for 23.1, 23.2, and 23.3 ([#&#8203;6623](`https://github.com/facebook/jest/pull/6623`))
- `[website]` Only test/deploy website if relevant files are changed ([#&#8203;6626](`https://github.com/facebook/jest/pull/6626`))
- `[docs]` Describe behavior of `resetModules` option when set to `false` ([#&#8203;6641](`https://github.com/facebook/jest/pull/6641`))

---


</details>




---

This PR has been generated by [Renovate Bot](https://renovatebot.com).
@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.

Interactive snapshot mode instantly finishes (and reports updates that are not performed)
5 participants