avajs / ava

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

`*.snap` files content should be the same regardless of test execution order #2311

Closed ehmicky closed 3 years ago

ehmicky commented 4 years ago

Description

When a test file has several parallel async tests and each of those tests call t.snapshot(), the resulting *.snap file is non-deterministic. The contents of the *.snap file depends on the order in which tests finished.

In such cases, tests pass correctly. However the *.snap files contents keep being updated, which means they are shown as dirty files by git status and they result in merge conflicts, especially in PRs.

Making *.snap file contents independent of test execution order would solve this issue.

Test Source

test.js

const test = require('ava')

const { promisify } = require('util')
const pSetTimeout = promisify(setTimeout)

test('one', async t => {
  await pSetTimeout(Math.random() * 1e3)
  t.snapshot('one')
})

test('two', async t => {
  await pSetTimeout(Math.random() * 1e3)
  t.snapshot('two')
})

Running ava -u on this file produces different *.snap depending on whether one or two finishes first.

$ ava -u test.js && sha256sum test.js.snap 

  2 tests passed

99b2f48bae8ee408643d0885635d18f1373ab17f2d3988160f5d5fc36e525f47  test.js.snap

$ ava -u test.js && sha256sum test.js.snap 

  2 tests passed

fbaf5758abca960c7b8e1daf7e06785cabed91c641d7daf595fb6939befd7daa  test.js.snap

$ ava -u test.js && sha256sum test.js.snap 

  2 tests passed

99b2f48bae8ee408643d0885635d18f1373ab17f2d3988160f5d5fc36e525f47  test.js.snap

Command-Line Arguments

Copy your npm build scripts or the ava command used:

ava -u test.js

Environment

Tell us which operating system you are using, as well as which versions of Node.js, npm, and AVA. Run the following to get it quickly:

$ node -e "var os=require('os');console.log('Node.js ' + process.version + '\n' + os.platform() + ' ' + os.release())"
Node.js v13.3.0
linux 5.3.0-24-generic
$ ava --version
2.4.0
$ npm --version
6.13.4
novemberborn commented 4 years ago

Oh interesting, I've never noticed that before. We should sort by test title, like we do for the Markdown files:

https://github.com/avajs/ava/blob/97571901faddf83d95dd1450ca2f131f51fb5856/lib/snapshot-manager.js#L107

novemberborn commented 4 years ago

We should sort by test title, like we do for the Markdown files:

We're looking at sorting by declaration order instead, see #2324. That said, this issue is primarily about having a stable sort order, and we could still use test titles for that.

ehmicky commented 3 years ago

Sweet, thanks!