avajs / ava

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

Remove `id` support in `t.snapshot()` #2669

Closed novemberborn closed 3 years ago

novemberborn commented 3 years ago

A presumably little known feature of the t.snapshot() assertion is that you can provide an ID for the snapshot. I doubt it's much used and it doesn't really fit with the mental model of of snapshots belonging to tests.

Let's remove it for AVA 4.

ninevra commented 3 years ago

I'd like to take a stab at implementing this, if that's alright? It'll simplify working on https://github.com/avajs/ava/discussions/2652.

Should there be an informative error message, or is the standard "The assertion message must be a string" sufficient?

novemberborn commented 3 years ago

Yea that'd be great @ninevra.

I'd like to raise an assertion failure when the option is passed, to aid people migrating from older AVA versions.

Note that we can make this change without requiring opt-in since we're now building towards AVA 4.