Add @Snapshot as built-in extension

[leonard84]

• Make Snapshotter production ready
• Group snapshots by class
• Introduce snapshotId for multiple snapshots
• Make normalization configurable
• Document Snapshotter
• Limit snapshotId length

[leonard84]

@skuzzle, maybe you want to review this. It is not intended to replace https://github.com/skuzzle/snapshot-tests but to provide a baseline of functionality.

[codecov]

Codecov Report

Attention: 25 lines in your changes are missing coverage. Please review.

Comparison is base (2c7db77) 80.44% compared to head (471bf72) 80.68%.
Report is 10 commits behind head on master.

Files	Patch %	Lines
...ock-core/src/main/java/spock/lang/Snapshotter.java	84.96%	18 Missing and 2 partials ⚠️
.../src/main/java/org/spockframework/util/IoUtil.java	58.33%	5 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1873      +/-   ##
============================================
+ Coverage     80.44%   80.68%   +0.23%     
- Complexity     4337     4363      +26     
============================================
  Files           441      441              
  Lines         13534    13674     +140     
  Branches       1707     1717      +10     
============================================
+ Hits          10888    11033     +145     
+ Misses         2008     2005       -3     
+ Partials        638      636       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[skuzzle]

I gave this a quick look and I think its pretty cool. The API is way simpler than what I built and yet allows to easily plugin additional functionality.

Some considerations and remarks:

• Though it feels a bit weird at first, I think it would be good to fail a test when a snapshot is first written or updated. For two reasons:
  1. If a user forgets to check in snapshot files, the build would fail on CI. Otherwise this might go unnoticed and the assertion would never actually be executed on CI
  2. Snapshot tests often suffer from random values contained in the asserted output. It makes sense to immediately run the test again to see whether it succeeds when actually comparing the new snapshot against the actual value
• One core value proposition of snapshot testing is, to make it easy to spot the differences when a test actually fails. I think the standard groovy assertion failures are not very helpful when comparing huge multi line strings. That is why I put quite a lot of effort into providing some nice diffs when an assertion fails. I know that it would be easily possible to plug this into your matchesSnapshot(BiFunction)method but it might be worth investigating whether to include some more sophisticated string diffing into the core
• I have built a detection for orhaned snapshots but I would not recommend going down this road. While its a very useful feature, it is nearly impossible to get it right because of the dynamic nature of snapshot creation.
• I like your approach of using explicit snapshot ids when having multiple snapshot assertions in one test. I went with auto-numbering snapshots within one test and it causes quite a lot of problems (e.g. when removing or temporarily commenting an assertion)

[leonard84]

Thanks for your feedback

Though it feels a bit weird at first, I think it would be good to fail a test when a snapshot is first written or updated. For two reasons:
If a user forgets to check in snapshot files, the build would fail on CI. Otherwise this might go unnoticed and the assertion would never actually be executed on CI

I don't see a connection between failing the test and adding the files to the commit.
Creating/updating the snapshots is a deliberate action and a user would expect to see changes in the files/new snapshots to be generated as that is the reason for running the update.
OTOH, I would be surprised at when the tests are failing.

Note: if a snapshot file is missing it will return the string <Missing Snapshot>, so unless the actual value is the same it will fail in the assertion.

Snapshot tests often suffer from random values contained in the asserted output. It makes sense to immediately run the test again to see whether it succeeds when actually comparing the new snapshot against the actual value

I can see that, however I don't think failing the test makes it obvious. I think I'll add a note to the documentation that one should re-run the tests after updating the snapshots to validate that they are stable and to be careful of dynamic values in general.

I think the standard groovy assertion failures are not very helpful when comparing huge multi line strings.

I agree, but I wanted to keep this feature small and not include any additional dependencies. It is also why I designed it to be extensible so that one can easily include better diffing. The other reason is that - at least for local development - the IntelliJ diff view is really helpful and alleviates the need for a full-blown diff in most cases.

I have built a detection for orhaned snapshots but I would not recommend going down this road. While its a very useful feature, it is nearly impossible to get it right because of the dynamic nature of snapshot creation.

Yeah, it is complicated, in Spock some snapshots are only generated/used when the build is run with a specific Groovy or Java version.

I like your approach of using explicit snapshot ids when having multiple snapshot assertions in one test. I went with auto-numbering snapshots within one test and it causes quite a lot of problems (e.g. when removing or temporarily commenting an assertion)

Stableness was one reason; the other was to offer the ability to validate that something hadn't changed between an action.

[AndreasTu]

@leonard84 Another nice feature would be, if we add a property to the @Snapshot annotation boolean writeActualFile() default false.
Which write the actual content into a file with suffix .actual, if the test fails and deletes the file if the test passes.

This would allow to open the actual content in another programm to check the file content, or use an external diff tool to compare and merge the "expected" content with the actual content. This can be helpfull for a bit more complicated file formats, which need an external tool.

[leonard84]

I've added the actual feature but decided to put the switch into the config and allow it to be controlled via system property for ad-hoc usage.

[AndreasTu]

Great. Thank you very much!