Jest: Add timestamp to snapshots

Created on 19 Aug 2017  路  4Comments  路  Source: facebook/jest

Do you want to request a feature or report a bug?
Request a feature/start a discussion

What is the current behavior?
Two commits updating the same snapshot can merge without conflicts but end up breaking tests.

What is the expected behavior?
Snapshot files could include a timestamp at the top which would cause a merge conflict if a commit did not update from the latest snapshot.

Something like this is what I have in mind:

// Jest Snapshot v1, https://goo.gl/fbAQLP
// Generated 2017-08-18T22:40:33+00:00

// ...

Anyone have any thoughts?

Most helpful comment

At the risk of bringing up a dead conversation, the reason we were suggesting this was because we were having tests mysteriously fail after two related PRs merged for our project using snapshots. It was as follows:

Alice makes a new test case (with a new snapshot), and Bob makes a change to how the component being snapshotted renders. Alice merges, but Bob doesn't rebase between Alice's merge and his. The result is that the new snapshot Alice made is broken as soon as Bob merges, but Bob never knew until the master branch had a failing test.

This was an attempt to work around this issue by forcing conflicts on folks who modified the same snapshot. The goal here wasn't to include time per-se, that was just a convenient way to generate a merge conflict and prevent broken tests. @cpojer If you have any other ideas of how to solve this problem, I'd be eager to hear them.

All 4 comments

Snapshot files should not contain time, and deleting snapshot files and recreating them should not result in a diff.

At the risk of bringing up a dead conversation, the reason we were suggesting this was because we were having tests mysteriously fail after two related PRs merged for our project using snapshots. It was as follows:

Alice makes a new test case (with a new snapshot), and Bob makes a change to how the component being snapshotted renders. Alice merges, but Bob doesn't rebase between Alice's merge and his. The result is that the new snapshot Alice made is broken as soon as Bob merges, but Bob never knew until the master branch had a failing test.

This was an attempt to work around this issue by forcing conflicts on folks who modified the same snapshot. The goal here wasn't to include time per-se, that was just a convenient way to generate a merge conflict and prevent broken tests. @cpojer If you have any other ideas of how to solve this problem, I'd be eager to hear them.

This is interesting but in most situations wouldn鈥檛 the build on Alice鈥檚 PR fail if it is set up to trigger a new PR build on any changes to the destination branch?

That is how we do it in my organization and prevent having the master branch be in a broken state.

This can work to solve the problem but doesn't scale to large organizations - if every change triggers new builds in every other PR, then you are stuck in a near-permanent state of waiting on builds to finish.

That is, while Bob waits for his PR to re-run after Alice's merge, he is also stuck behind (or blocking) Charlie. If Charlie's jobs happen to finish first, Bob has now been blocked on two full builds even though he made no changes to his PR. Repeat ad nauseam for however many developers you have working on the same repo, and you eventually end up with a PR that was ready for merge waiting an extremely long amount of time before it can actually go in.

This can be mitigated somewhat by having _really smart_ change detection to see if changes in master need to trigger a run of _your_ PR based on the changes in it, but that gets more and more difficult the larger and more interconnected your repo is.

What we have done in our organization is trigger jobs on every push and having those jobs shadow the destination branch. This scales better, but unfortunately opens us up to the (admittedly edge) case described here.

Was this page helpful?
0 / 5 - 0 ratings