Tests: Extend TestCatchpointAfterTxns to catch catchpoint write corruption#4818
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4818 +/- ##
==========================================
- Coverage 54.63% 54.62% -0.02%
==========================================
Files 417 417
Lines 53734 53734
==========================================
- Hits 29358 29350 -8
- Misses 21940 21941 +1
- Partials 2436 2443 +7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| defer dl.Close() | ||
| // Exercises interactions between transaction evaluation and catchpoint | ||
| // generation to confirm catchpoints include expected transactions. | ||
| t.Run("chunks", func(t *testing.T) { |
There was a problem hiding this comment.
Are we gaining something from pushing things into a t.Run? It obscures the diff, and I don't think it has the isolation properties you might be hoping for anyway.
There was a problem hiding this comment.
@jannotti It's subjective - I think there are 2 discrete test cases here. I felt they can be left under the same test name (TestCatchpointAfterTxns) and separated by t.Run.
The PR's initial commit shows everything inline as 1 test case without t.Run. As mentioned in the PR description, I'm willing to remove t.Run if reviewers feel strongly.
2 participants
Extends catchpoint tests to help catch regressions:
testNewLedgerFromCatchpointto confirm the writer and reader merkle tries match by comparing merkle trie stats.TestCatchpointAfterTxnsisolating a scenario that fails prior to cow: always process KvMods to add OldData to mods when building StateDeltas #4804.Notes:
DoubleLedger. As is, the PR contains a kludgey if-statement (Skip invariant check for tests using mocks). Even as is, I think the PR presents a value add.