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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print output when snapshots are updated #1583

Closed
jednano opened this issue Nov 8, 2017 · 8 comments
Closed

Print output when snapshots are updated #1583

jednano opened this issue Nov 8, 2017 · 8 comments

Comments

@jednano
Copy link
Contributor

jednano commented Nov 8, 2017

Description

Notify when a snapshot update is available to match Jest snapshot behavior.

New snapshot summary (bold in green 馃摋)

  • 1 snapshot written in 1 test suite.

Obsolete/failed snapshot summary (bold in red 馃摍)

  • 1 obsolete snapshot found, press u to remove them.
  • 1 obsolete snapshot file removed.
  • 1 snapshot test failed in 1 test suite. Inspect your code changes or press u to update them.

@novemberborn
Copy link
Member

Besides familiarity with Jest, could you elaborate on why you'd find this information useful?

@jednano
Copy link
Contributor Author

jednano commented Nov 13, 2017

Sometimes I add or change a test and it gets appended to the end of the snapshot Markdown file. If I hit u then it refreshes the whole Markdown file and the snapshot that was previously at the end of the file is now somewhere in the middle. This isn't obvious, because there's no messaging that tells me my snapshots need to be updated. Technically, the snapshot itself perhaps doesn't need to be updated, but the Markdown file does (at least). I need some kind of indicator to tell me it's time to hit u so I can ensure my Markdown file won't change with a subsequent test run. In other words, the watch command shouldn't be yielding a different Markdown file than if I were to hit u or do a normal, non-watch test run. Does that make sense?

@novemberborn
Copy link
Member

Ah. The Markdown file is appended to, rather than rewritten, for performance reasons. It's not used by AVA though. I don't know how big of a problem this is, but then to be honest I tend to explicitly update snapshots before committing them.

This isn't obvious, because there's no messaging that tells me my snapshots need to be updated. Technically, the snapshot itself perhaps doesn't need to be updated, but the Markdown file does (at least).

They don't need to be updated manually, though I appreciate you're trying to avoid extraneous changes in the future when you're explicitly updating snapshots for unrelated reasons.

@jednano
Copy link
Contributor Author

jednano commented Nov 14, 2017

Yeah I realize the Markdown files are not used by AVA, but AVA does produce two different results for the Markdown file, depending on watch vs. normal run, as you mentioned, for performance reasons. That's fine. I just wish there were some notification that it needs a hard re-write (an update) in the watch scenario. You explicitly update snapshots before committing and so do I for this very reason, but if it were more clear, we wouldn't have to be so trigger happy w/ the updates.

@novemberborn
Copy link
Member

Right so you're saying you'd like a reminder from AVA that snapshots were modified, so you can immediately rebuild them. That makes sense I think.

Note though that we shouldn't communicate "and now update your snapshots", since everything is working fine. This is for those of us who can't abide the thought of future churn in the Markdown files 馃槈

@jednano
Copy link
Contributor Author

jednano commented Nov 14, 2017

Agreed. That would be misleading. Perhaps a message like:

Appended 2 snapshots to Markdown files. Press u to update.

I'm not sure how to be any more concise. The more verbose version would say "...to update/sync with snapshot order."

@jednano jednano changed the title Notify when a snapshot update is available Notify when a Markdown update is available Nov 14, 2017
@novemberborn
Copy link
Member

"Updated 2 snapshots" would be sufficient I think. Folks like us will catch on and know what to do, especially when we land #1525.

@novemberborn novemberborn changed the title Notify when a Markdown update is available Print output when snapshots are updated Nov 18, 2017
@novemberborn novemberborn added enhancement new functionality help wanted and removed question labels Nov 18, 2017
@novemberborn
Copy link
Member

Both due to the age of this issue, and the state of our reporters, I've decided to roll this into #2501.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants