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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store snapshots by test declaration order #2324

Closed
sramam opened this issue Dec 30, 2019 · 5 comments · Fixed by #2610
Closed

Store snapshots by test declaration order #2324

sramam opened this issue Dec 30, 2019 · 5 comments · Fixed by #2610

Comments

@sramam
Copy link
Contributor

sramam commented Dec 30, 2019

Description

Please consider not sorting snapshots when they are recorded - especially to the markdown version.

My use case

I use ava with snapshots for testing "pure-ish" functions that are composed into larger workflows.
Meaning while the functions have side effects, they are testable with input/output expectations.
Snapshots are ideal for this.

However, since these are functions with side-effects, for proper testing, they have to be run in serial order to simulate the workflow. This allows the functions to be tested in isolation, while
building upon previous steps. The cost of setup of partial workflows is significant enough that it's not practical to think of think of running each function as a test suite.

Snapshots are a great way to capture the expectations and track deviations over time.
However in snapshot-manager.js these recordings are being sorted in title-order.

Now my ability to manually inspect the output is severely impacted, requiring all kinds of
up-and-down scrolling. Some of these are API calls with 10s or even 100s of lines of data.
Sometimes, out of sheer frustration, I put numbers in the test titles, but that turns out to cause
other problems in complex refactoring/requirement-change situations.

Options I can think of:

  1. Not sorting - however, this seems to be exactly counter of this comment on #2311
  2. Sorting only the snap-files, but not the markdown files
  3. Sorting parallel tests, but not serial tests.
  4. Providing a flag/option to not sort the markdown files.
  5. Storing snapshots in order they are encountered in the test files.

Strikes me that option 5 would seem to solve all problems (including #2311?), allow for side-by-side scrolling of test file and markdown, without requiring any configuration.

Really unsure of the implementation complexity though.

@sramam sramam changed the title snapshots should not be so snapshots should not be sorted Dec 30, 2019
@sramam sramam changed the title snapshots should not be sorted do not sort snapshot records Dec 30, 2019
@novemberborn novemberborn changed the title do not sort snapshot records Store snapshots by test declaration order Jan 2, 2020
@novemberborn
Copy link
Member

5. Storing snapshots in order they are encountered in the test files.

Strikes me that option 5 would seem to solve all problems (including #2311?), allow for side-by-side scrolling of test file and markdown, without requiring any configuration.

Yup, we should do this. Thanks for bringing it up.

It should be possible to assign incremental IDs as tests are declared, and use those to sort the snapshots.

@sramam
Copy link
Contributor Author

sramam commented Jan 2, 2020

ooh - that's so interesting that you mention incremental ids.
Perhaps I should open a new issue for this, but can't resist asking:

Could that also be used to order the verbose reports on the CLI?
That'd be so valuable when one has a large number of tests
(200+ in my case - this is the final integration module, made up of many smaller modules)

@novemberborn
Copy link
Member

Perhaps I should open a new issue for this, but can't resist asking:

Could that also be used to order the verbose reports on the CLI?
That'd be so valuable when one has a large number of tests
(200+ in my case - this is the final integration module, made up of many smaller modules)

Yea, but we need to rewrite the reporters anyhow.

@sramam
Copy link
Contributor Author

sramam commented Jan 3, 2020

Yea, but we need to rewrite the reporters anyhow.

as part of this issue?

I'd be interested understanding the thinking behind changes to reporters. A quick search didn't reveal anything obvious.

@novemberborn
Copy link
Member

as part of this issue?

No.

I'd be interested understanding the thinking behind changes to reporters. A quick search didn't reveal anything obvious.

See #2217.

novemberborn added a commit that referenced this issue Dec 6, 2020
Ensures that *.snap files are deterministic by sorting entry blocks by the hash of their test name or id.

Ensures that *.md snapshot report files are sorted in as close to declaration order as is reasonably possible.

Closes #2311
Closes #2324

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>
Co-authored-by: Mark Wubben <mark@novemberborn.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants