ava
ava copied to clipboard
First parameter for "snapshot" assertion should be called "value" for consistency
Currently, it is called "expected": https://github.com/avajs/ava/blob/main/docs/03-assertions.md#snapshotexpected-message
However, that is inconsistent with other assertion APIs like "is" where parameters are named "value" and "expected": https://github.com/avajs/ava/blob/main/docs/03-assertions.md#isvalue-expected-message
In the case of "snapshot", the expected data is represented by the previously-captured snapshot and the test data (often called "actual") is what gets passed to the function via the first parameter.
Sure, could change that. To value
? There's the documentation and the TypeScript definition, and then the implementation itself.
I prefer "actual "and "expected" as this project uses in the TypeScript declaration file: https://github.com/avajs/ava/blob/6b63b1bc14a1aaa7d4ee4f2201f901624419c677/types/assertions.d.ts#L184
However, this project seems to use the terms "value" and "expected" in the documentation as I link to here. That is an inconsistency which should probably also be addressed. If you need me to open another issue for that, I can.
However using the name "expected" with the "snapshot" assertion is backwards and misleading and therefore seems more urgent.
Here's an issue for the documentation vs. declaration mismatch: https://github.com/avajs/ava/issues/2938
Sounds good to me. Would you like to open a PR @DavidAnson?
Please don't wait for me to send a PR, it's unlikely I'll have time for this soon.
Looks like snapshot()
is fundamentally different from other assertions and thus its currently document param name is correct.
Please correct me if I'm wrong: When running tests (common scenario), the input to the snapshot function is the actual value observed when running the test. The snapshot function then reads the snapshot file to determine the expected value for that test and diffs the two values.
The situation is different when running in --update-snapshots
mode to capture a new snapshot, but that should not be the focus of the API naming, in my opinion.
I got that one backwards. I also don't use snapshots.
Looking at this now I agree actual
makes more sense. But I need to have a closer look, see how the diffs are presented and so forth. Simply renaming may not add clarity.