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

tests: add catchpoint downloading/parsing to e2e catchup tests #6224

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cce
Copy link
Contributor

@cce cce commented Jan 9, 2025

Summary

While working on #6214 and #6177, it would have been nice to be able to download catchpoint files and decode them inside E2E tests. This exports the catchpoint chunk formats (and msgp unmarshal code), so they can be used by the e2e tests to decode and assert catchpoint file snapshot contents.

While renaming the chunk types so they would be exported, their new names were carefully chosen to preserve alphabetical ordering and make the diff in msgp_gen.go easy to review (to see it is a name change only, no other changes to decode/encode logic).

Test Plan

Existing E2E tests should pass, with additional assertions added.

@cce cce added the Enhancement label Jan 9, 2025
@cce
Copy link
Contributor Author

cce commented Jan 9, 2025

Running these assertions on TestStateProofInReplayCatchpoint but with the fix in #6214 commented out, it caught the issue:

    catchpointCatchup_test.go:539: Downloading catchpoint file for round 28 from http://127.0.0.1:59582/v1/test-v1/ledger/s
    catchpointCatchup_test.go:545: Downloaded catchpoint file for round 28, size 10240 bytes
    catchpointCatchup_test.go:552: tar filename: content.msgpack, size 312
    catchpointCatchup_test.go:552: tar filename: stateProofVerificationContext.msgpack, size 125
    catchpointCatchup_test.go:552: tar filename: balances.1.msgpack, size 511
    catchpointCatchup_test.go:560: chunk 1 has balances: 4, kvs: 0, online accounts: 0, onlineroundparams: 0
    catchpointCatchup_test.go:552: tar filename: balances.2.msgpack, size 3936
    catchpointCatchup_test.go:560: chunk 2 has balances: 0, kvs: 0, online accounts: 17, onlineroundparams: 0
    catchpointCatchup_test.go:552: tar filename: balances.3.msgpack, size 906
    catchpointCatchup_test.go:560: chunk 3 has balances: 0, kvs: 0, online accounts: 0, onlineroundparams: 15
    fixture.go:120:
                Error Trace:    /Users/cce/ga9/test/e2e-go/features/catchup/catchpointCatchup_test.go:529
                                                        /Users/cce/ga9/test/e2e-go/features/catchup/stateproofsCatchup_test.go:98
                Error:          Not equal:
                                expected: 8
                                actual  : 15
                Test:           TestStateProofInReplayCatchpoint
                Messages:       online round params chunk should be same size as lookback 8

Copy link

codecov bot commented Jan 13, 2025

Codecov Report

Attention: Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 51.85%. Comparing base (c60db8d) to head (e16e00e).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
ledger/catchupaccessor.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6224      +/-   ##
==========================================
- Coverage   51.86%   51.85%   -0.02%     
==========================================
  Files         643      643              
  Lines       86382    86382              
==========================================
- Hits        44806    44796      -10     
- Misses      38714    38724      +10     
  Partials     2862     2862              

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

@cce cce marked this pull request as ready for review January 14, 2025 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant