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

Should verify snapshots came from supported snapshottest version #143

Open
medmunds opened this issue Oct 3, 2020 · 1 comment
Open

Comments

@medmunds
Copy link
Collaborator

medmunds commented Oct 3, 2020

Currently, the generated snapshot file includes "v1" in a header comment, indicating v1 of the snapshot file format. (I believe this approach was borrowed from jest-snapshot.)

Current snapshot file: (ignoring Python 2 compatibility lines)

# snapshottest: v1 - https://goo.gl/zC4yUc
from snapshottest import Snapshot


snapshots = Snapshot()

snapshots['TestDemo::test_api_me 1'] = {
    'url': '/me'
}

However, nothing actually checks that "v1" anywhere. If we change the snapshot file format in the future, that could lead to confusing errors or incorrect results if developers on a team somehow have different versions of snapshottest installed. (Which, you know, development teams should really try to avoid, but we don't control that.)

In particular, if developer A has snapshottest v0.6.0 installed, and developer B updates snapshots using some future, incompatible version, developer A's snapshottest won't be able to detect that case. (So we should really add version checking soon, not wait until we need to change the file format.)

Rather than trying to parse version numbers out of source comments, I suggest we use a module level variable, which could be easily checked in SnapshotModule.load_snapshots.

Proposed snapshot file:

# generated by snapshottest: https://pypi.org/project/snapshottest/
from snapshottest import Snapshot


_snapshottest_format = 1
snapshots = Snapshot()

snapshots['TestDemo::test_api_me 1'] = {
    'url': '/me'
}

(I've also changed the project link from the opaque goo.gl link shortener to PyPI. The PyPI project link should be stable for the lifetime of this package, allows moving the project home if ever desired, and is less mysterious.)

Some variations:

  • Rather than module level _snapshottest_format = 1, pass the version to the Snapshot constructor: snapshots = Snapshot(format=1). Pro: shorter/cleaner. Con: prevents format changes that would eliminate the Snapshot class. (I can't imagine one now, but this seems overly restrictive.)
  • Rather than a file format version constant, use the snapshottest release version that wrote the file (_snapshottest_version = (0, 6, 0)). Pro: guarantees we never accidentally release a format change but forget to bump the format version. Con: unnecessary churn for the vast majority of releases that won't change the format, particularly if users have CI that updates their snapshots.

Also, for reference, Jest's version checking tests include their error message text, which we might want to borrow.

@paulmelnikow
Copy link
Collaborator

Great idea. I'd also consider backporting this change and releasing it as 0.6.1 for people who can't or don't make the leap to 1.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants