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

Deleting a snap file runs all tests #1511

Closed
wmertens opened this Issue Sep 7, 2017 · 12 comments

Comments

Projects
None yet
5 participants
@wmertens
Copy link

wmertens commented Sep 7, 2017

Description

I think this is a nice workflow:

  • write complex data processing code
  • feed it example data in tests with t.snapshot()
  • inspect result .md file in editor tab
  • if snapshot is no longer good, remove .snap file so it gets regenerated

👉 this runs all the tests again, instead of just the test using the snap file 😢

ava v0.22

@novemberborn

This comment has been minimized.

Copy link
Member

novemberborn commented Sep 10, 2017

Oh this is a bug!

The watcher is observing .snap files:

paths = ['package.json', '**/*.js', '**/*.snap'];

However it fails to recognize them as source files:

mixedPatterns = ['package.json', '**/*.js'].concat(mixedPatterns);

This means that it can't trace the removal of the file to the test file that created it, so it reruns all tests.

Those arrays should have the same items, or perhaps we can even define the array as a constant. Just need to make sure it's not accidentally modified.

@wmertens

This comment has been minimized.

Copy link
Author

wmertens commented Sep 10, 2017

Am I reading correctly that if you have custom paths, the watcher won't even watch .snap files? Those only get added if paths.length === 0?

@novemberborn

This comment has been minimized.

Copy link
Member

novemberborn commented Sep 11, 2017

Am I reading correctly that if you have custom paths, the watcher won't even watch .snap files? Those only get added if paths.length === 0?

Yes, but that's the same for the package.json and **/*.js paths. There has to be a way of overriding patterns (especially when they contain **), and the easiest approach is to make you restate what you need.

@wmertens

This comment has been minimized.

Copy link
Author

wmertens commented Sep 11, 2017

I would argue that package.json and .snap files are internal to AVA, and should therefore always be added. Doesn't the ignore list take precedence?

@mfainshtein2

This comment has been minimized.

Copy link

mfainshtein2 commented Mar 19, 2018

I would like to take a crack at this issue. Where should I start looking to find the cause of this issue?

@novemberborn

This comment has been minimized.

Copy link
Member

novemberborn commented Mar 19, 2018

@mfainshtein2 as a first step try reproducing the issue. #1511 (comment) has a proposed solution which hopefully is still up to date. You can find a previous attempt to fix this in #1515.

@mfainshtein2

This comment has been minimized.

Copy link

mfainshtein2 commented Mar 26, 2018

@novemberborn, Your proposed solution seems to have worked, only the test that had its .snap file deleted got rerun but it does show the following message if the other test has a mismatch. Is this intentional?
image

@novemberborn

This comment has been minimized.

Copy link
Member

novemberborn commented Mar 26, 2018

it does show the following message if the other test has a mismatch. Is this intentional?

It is.

mfainshtein2 pushed a commit to mfainshtein2/ava that referenced this issue Mar 26, 2018

mfainshtein2
avajs#1511 - Added '**/*.snap' to mixedPatterns to keep the watcher f…
…rom running all tests when one snap file is deleted.
@IssuehuntBot

This comment has been minimized.

Copy link

IssuehuntBot commented Dec 22, 2018

@IssueHuntFest has funded $30.00 to this issue. See it on IssueHunt

@itaisteinherz

This comment has been minimized.

Copy link
Contributor

itaisteinherz commented Feb 8, 2019

Since #1751 wasn't merged, can I take a shot at fixing this?

@novemberborn

This comment has been minimized.

Copy link
Member

novemberborn commented Feb 8, 2019

Yea go for it @itaisteinherz!

@novemberborn novemberborn added assigned and removed has pr labels Feb 8, 2019

novemberborn added a commit that referenced this issue Feb 10, 2019

@IssuehuntBot

This comment has been minimized.

Copy link

IssuehuntBot commented Feb 10, 2019

@sindresorhus has rewarded $27.00 to @itaisteinherz. See it on IssueHunt

  • 💰 Total deposit: $30.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $3.00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment