Skip to content

Rework golden tests #711

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jorisdral
Copy link
Collaborator

@jorisdral jorisdral commented May 8, 2025

Previously, we were encoding each snapshot type in the context of a full
metadata type. However, if one field somewhere down in the snapshot metadata
changes, then the encoding of the full metadata stays the same except for that
one field further down. As such, this commit changes the test setup so that we
now encode (and golden-test) each type involved in the snapshot metadata
separately.

We also add a new EnumGolden class for enumerating values of snapshot types.
With the more direct tests and EnumGolden, it's easier to prevent
combinatorial explosion.

There is now also a new test that checks whether there are no missing or unexpected files in the golden data directory.

Example output when there is an unexpected file:

❯ cabal run lsm-tree-test -- -p "prop_noUnexpectedOrMissingGoldenFiles"
lsm-tree
  Test.Database.LSMTree.Internal.Snapshot.Codec.Golden
    prop_noUnexpectedOrMissingGoldenFiles: FAIL
      *** Failed! Falsified (after 1 test):
      Found unexpected files: fromList ["unexpected"]
      Delete the unexpected files manually from test/golden-file-data/snapshot-codec
      Use --quickcheck-replay="(SMGen 1489992322676650058 4671765810150648345,0)" to reproduce.

1 out of 1 tests failed (0.01s)

Example output when there is a missing file:

❯ cabal run lsm-tree-test -- -p "prop_noUnexpectedOrMissingGoldenFiles"
lsm-tree
  Test.Database.LSMTree.Internal.Snapshot.Codec.Golden
    prop_noUnexpectedOrMissingGoldenFiles: FAIL
      *** Failed! Falsified (after 1 test):
      Missing expected files: fromList ["NominalCredits.A.snapshot.golden"]
      Run the golden tests to regenerate the missing files
      Use --quickcheck-replay="(SMGen 11771750078239108519 7040896247146249591,0)" to reproduce.

1 out of 1 tests failed (0.01s)

Comment on lines +226 to +264
instance EnumGolden SnapshotMetaData where
singGolden = SnapshotMetaData singGolden singGolden singGolden singGolden singGolden
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern of applying a constructor to a string of singGoldens looks like it could be generically derived. Maybe I should add a TODO

Comment on lines +243 to +244
nameGolden :: Typeable a => Proxy a -> Annotation -> String
nameGolden p ann = map spaceToUnderscore (show $ typeRep p) ++ "." ++ ann
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The downside of using typeRep is that the name of the golden file/test changes when the type name changes. In some cases, we might want to rename the type without affecting the name, so maybe nameGolden should be a member of EnumGolden?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The typeRep approach can still be the default, but the upside is that you can change the name per instance

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this be an issue? Seems easier to just recreate the relevant golden files than to maintain a type-name mapping forever.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe not, it's just something that came to mind. I'm fine with leaving it as is too

jorisdral added 4 commits May 8, 2025 17:18
Previously, we were encoding each snapshot type in the context of a full
metadata type. However, if one field somewhere down in the snapshot metadata
changes, then the encoding of the full metadata stays the same except for that
one field further down. As such, this commit changes the test setup so that we
now encode (and golden-test) each type involved in the snapshot metadata
separately.

We also add a new `EnumGolden` class for enumerating values of snapshot types.
With the more direct tests and `EnumGolden`, it's easier to prevent
combinatorial explosion.
Example output when there is an unexpected file:

```
❯ cabal run lsm-tree-test -- -p "prop_noUnexpectedOrMissingGoldenFiles"
lsm-tree
  Test.Database.LSMTree.Internal.Snapshot.Codec.Golden
    prop_noUnexpectedOrMissingGoldenFiles: FAIL
      *** Failed! Falsified (after 1 test):
      Found unexpected files: fromList ["unexpected"]
      Delete the unexpected files manually from test/golden-file-data/snapshot-codec
      Use --quickcheck-replay="(SMGen 1489992322676650058 4671765810150648345,0)" to reproduce.

1 out of 1 tests failed (0.01s)
```

Example output when there is a missing file:

```
❯ cabal run lsm-tree-test -- -p "prop_noUnexpectedOrMissingGoldenFiles"
lsm-tree
  Test.Database.LSMTree.Internal.Snapshot.Codec.Golden
    prop_noUnexpectedOrMissingGoldenFiles: FAIL
      *** Failed! Falsified (after 1 test):
      Missing expected files: fromList ["NominalCredits.A.snapshot.golden"]
      Run the golden tests to regenerate the missing files
      Use --quickcheck-replay="(SMGen 11771750078239108519 7040896247146249591,0)" to reproduce.

1 out of 1 tests failed (0.01s)
```
@jorisdral jorisdral force-pushed the jdral/rework-golden-combinatorial-explosion branch from fe14e01 to 66d80ec Compare May 8, 2025 15:19
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

Successfully merging this pull request may close these issues.

2 participants