Skip to content

test: fix flaky TestLedgerBlockHeaders UpgradeState check#6382

Merged
gmalouf merged 1 commit intoalgorand:masterfrom
cce:flaky-TestLedgerBlockHeaders
Jul 15, 2025
Merged

test: fix flaky TestLedgerBlockHeaders UpgradeState check#6382
gmalouf merged 1 commit intoalgorand:masterfrom
cce:flaky-TestLedgerBlockHeaders

Conversation

@cce
Copy link
Copy Markdown
Contributor

@cce cce commented Jul 15, 2025

Summary

TestLedgerBlockHeaders was failing intermittently with "block branch512 incorrect" instead of the expected "UpgradeState mismatch" error. This happened because:

  1. The test randomly picks a "wrong" consensus version from a map (non-deterministic iteration)
  2. The original code only checked if the wrong version supports SHA512, but didn't consider mismatches with the current protocol
  3. When there was a mismatch (current protocol has/doesn't have SHA512 support vs wrong version expects/doesn't expect it), the Branch512 validation would fail before reaching the intended UpgradeState check

The fix explicitly handles both mismatch cases to ensure the test reaches the intended validation error.

Test Plan

Ran with go test -run TestLedgerBlockHeaders -count=100 to reproduce and fix.

@cce cce added the Bug-Fix label Jul 15, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Jul 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.44%. Comparing base (da10cb5) to head (55b64ee).
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6382      +/-   ##
==========================================
- Coverage   50.60%   50.44%   -0.16%     
==========================================
  Files         660      653       -7     
  Lines      110483   110415      -68     
==========================================
- Hits        55913    55702     -211     
- Misses      51713    51855     +142     
- Partials     2857     2858       +1     

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

@gmalouf gmalouf merged commit 343d3dd into algorand:master Jul 15, 2025
32 of 33 checks passed
@cce cce deleted the flaky-TestLedgerBlockHeaders branch July 16, 2025 02:48
@cce cce requested a review from Copilot July 16, 2025 02:49
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a flaky test TestLedgerBlockHeaders that was intermittently failing with "block branch512 incorrect" instead of the expected "UpgradeState mismatch" error. The issue occurred due to non-deterministic consensus version selection and insufficient handling of SHA512 field mismatches between protocol versions.

  • Adds explicit handling for both SHA512 support mismatch scenarios
  • Ensures the Branch512 field is correctly set to match the wrong version's expectations
  • Prevents premature validation failure on Branch512 before reaching the intended UpgradeState check

Comment thread ledger/ledger_test.go
Comment thread ledger/ledger_test.go
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