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

catchpoints: move logic in readDatabaseStep to reusable catchpointOnlineAccountsIterWrapper #6253

Merged
merged 7 commits into from
Feb 13, 2025

Conversation

cce
Copy link
Contributor

@cce cce commented Feb 12, 2025

Summary

Fix label generation issue detected in nightly e2e tests after merging #6240. Makes the logic reusable for both catchpoint label trackers (CatchpointTracking: 1) and catchpoint file generators (CatchpointTracking: 2) as well as tests.

Test Plan

Existing tests pass now — additional tests to build on #6224 and further assert contents of catchpoint files will follow in a separate PR.

@cce cce added the Bug-Fix label Feb 12, 2025
@cce cce changed the title catchpoints: calculate onlineAccountsHash in catchpointFileWriter.readDatabaseStep catchpoints: calculate online hashes in catchpointFileWriter.readDatabaseStep Feb 12, 2025
algorandskiy
algorandskiy previously approved these changes Feb 12, 2025
ledger/catchpointtracker.go Outdated Show resolved Hide resolved
ledger/catchpointtracker_test.go Outdated Show resolved Hide resolved
@cce cce changed the title catchpoints: calculate online hashes in catchpointFileWriter.readDatabaseStep catchpoints: move logic in readDatabaseStep to reusable catchpointOnlineAccountsIterWrapper Feb 13, 2025
Copy link

codecov bot commented Feb 13, 2025

Codecov Report

Attention: Patch coverage is 76.92308% with 6 lines in your changes missing coverage. Please review.

Project coverage is 51.78%. Comparing base (2f07e60) to head (bc0053d).
Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
ledger/catchpointfilewriter.go 76.00% 3 Missing and 3 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6253   +/-   ##
=======================================
  Coverage   51.77%   51.78%           
=======================================
  Files         644      644           
  Lines       86494    86511   +17     
=======================================
+ Hits        44781    44797   +16     
+ Misses      38843    38841    -2     
- Partials     2870     2873    +3     

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

@algorandskiy algorandskiy merged commit 8fce49c into algorand:master Feb 13, 2025
22 checks passed
Copy link
Contributor

@jannotti jannotti left a comment

Choose a reason for hiding this comment

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

Makes sense to me. Tests passing is probably the best possible code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants