bugfixes in unittests/snapshot_tests.cpp

[cc32d9]

This is a copy of EOSIO/eos#10909
The fix is applicable to Mandel, as you backported the improved unit tests

This needs to be merged into Mandel, as it helps generating the snapshots for distributions like WAX:

cc32d9/wax2.1@b5ebb8f

[swatanabe]

I don't understand the point of this change. Changing from continue to break makes no difference at all, since it's the last iteration of the loop. The snapshot generation code is intended to be used in a very specific way: when adding a new snapshot version, generate a snapshot and save it forever to make sure that future versions of the software can load it.

snapshot_tests.cpp.diff is a patch against 1.8, not any other version. The whole point of the test is to verify that the snapshots generated by 1.8 can be loaded by later versions. It isn't correct to call the patch obsolete.

[cc32d9]

I used this code to create v2 to v5 snapshots for WAX: https://github.com/cc32d9/wax2.1/tree/master/unittests/snapshots

in the original version with "continue", I set the current version to v2, and this loop doesn't break, which results in a test fail, so I can't produce a v2 snapshot. And so on. With the fixed version, I was able to create all required snapshots.

Correspondingly, I needed a patched version of snapshot_tests.cpp.diff because I didn't have a way to generate the required blocks log without this utility.

So, whenever a new blockchain with a customized set of intrinsics starts, it will need to go through this procedure again and produce a new set of files.

[cc32d9]

More precisely, I only had v2 snapshots at hand: https://github.com/cc32d9/wax2.0/tree/master/unittests/snapshots

[swatanabe]

You didn't generate v2, v3, v4, v5 snapshots. You generated a bunch of v5 snapshots with different file names.

[swatanabe]

In general, it only makes sense to save a snapshot when there is an actual version of the software that produces snapshots in that format, which is what the test is designed to handle.

[cc32d9]

exactly, but 2.0 did not have a well defined procedure for snapshot creation

[swatanabe]

The right way to handle this is to take the test case and port it into a copy of 2.0, removing the trailing snapshot versions that don't exist in 2.0. Similarly, for snapshot_tests.cpp.diff, you need apply it to a copy of 1.8. If you're creating the snapshots with the current version of the software, you might as well not bother. Just remove the tests that you don't have snapshots for.

[cc32d9]

the goal is to keep WAX as less deviated from upstream as possible

[swatanabe]

If WAX's snapshot format is incompatible, then some deviation is unavoidable.

[swatanabe]

Adjusting the list of snapshots + removing an obsolete tests seems pretty manageable to me.

[cc32d9]

I'll get back to this topic when I start porting WAX to Mandel

[swatanabe]

I think, in the long term the correct fix is to make it so that new protocol features do not need to change the snapshot format. Adding a map { protocol feature -> arbitrary data } to the snapshot representation of global_property_object would handle all the cases that I am aware of.