avajs / ava

Node.js test runner that lets you develop with confidence 🚀
MIT License
20.74k stars 1.41k forks source link

Print output when snapshots are updated #1583

Closed jednano closed 4 years ago

jednano commented 6 years ago

Description

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

New snapshot summary (bold in green 📗)

Obsolete/failed snapshot summary (bold in red 📙)

novemberborn commented 6 years ago

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

jednano commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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."

novemberborn commented 6 years ago

"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 commented 4 years ago

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