testing: assert ErrorContains/ErrorIs instead of Error #6512
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Phase 1 of error assertion migration: Replace all require.Error, assert.Error, and a.Error calls with errorcontains.CaptureError to capture actual error types and messages during test runs. This enables analysis of what errors are actually being checked, so they can be converted to more specific assertions like ErrorContains, ErrorIs, or ErrorAs in Phase 2. 108 files changed, ~550 replacements.
Use the go-lcss library for efficient multi-string longest common substring computation instead of the flawed iterative approach.
Convert 345 errorcontains.CaptureError calls to specific assertions: - 36 removed (kept existing ErrorIs) - 8 converted to ErrorIs (from Equal with sentinel) - 30 converted to ErrorContains (from Contains) - 273 standalone converted to ErrorContains with LCS substrings 210 calls remain for manual review.
Continue converting CaptureError/CaptureErrorAssert calls to specific assertions based on captured error data. This batch includes: - agreement: bundle, certificate, vote tests - crypto: batchverifier, multisig tests - data: participationRegistry, bookkeeping, pools tests - data/transactions: verify, txnBatch, verifiedTxnCache tests - ledger: apply/payment, boxtxn, ledger tests - network: p2pMetainfo tests - node: follower_node tests - rpcs: txSyncer tests Also fixes path normalization in aggregate/phase3 tools to handle both go-algorand and errorcontains repo names.
Convert all non-E2E CaptureError/CaptureErrorAssert calls to specific error assertions across 18 test files: - agreement: proposalTracker_test.go, pseudonode_test.go - cmd/goal: node_test.go - data: ledger_test.go - ledger: acctupdates_test.go, ledger_test.go - network: addr_test.go, wsNetwork_test.go - stateproof: stateproof_test.go - tools: block-generator, dnssec, resolveController, typeAnalyzer - util: rateLimit_test.go Each assertion now checks for specific error message substrings rather than just verifying an error occurred. Two cases use require.Error with comments where error messages vary by platform or external factors. Also removes unused errorcontains imports.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6512 +/- ##
==========================================
- Coverage 47.87% 47.68% -0.19%
==========================================
Files 662 655 -7
Lines 87934 87849 -85
==========================================
- Hits 42095 41895 -200
- Misses 43076 43182 +106
- Partials 2763 2772 +9 ☔ View full report in Codecov by Sentry. |
Replace meaningless struct dump fragments (from LCS algorithm) with actual error message substrings in ErrorContains assertions: - ledger_test.go: Fix assertions for invalid tx type, overspend, no signature, applyData mismatch, and overlapping lease - bundle_test.go: Fix duplicated equivocation vote assertion - Remove unused errorcontains imports after converting all calls
- crypto/multisig_test.go: fix parameter order in ErrorContains - config/config_test.go: remove machine-specific temp paths - netdeploy/networkTemplates_test.go: remove absolute path prefix - network/p2p/p2p_test.go: use generic error prefix for all test cases - ledger/double_test.go: remove overly-specific logic eval error check - tools/network/resolver_test.go: remove machine-specific IPv6 resolver
The `, data` substring was matching part of a struct dump in the error message, not a meaningful error condition. These tests should assert `overspend` which is the actual error type being returned.
- Remove backslash-escaping in raw strings (quotes need no escaping) - Revert generic error checks where various errors can occur - Fix parameter order in ErrorContains calls - Remove dynamic values from error substrings (round numbers, etc.)
The round, step, and dbRound values vary between test runs, so only check for the stable error message prefix.
jannotti
reviewed
Dec 10, 2025
| txgroup := []transactions.SignedTxn{stbad} | ||
| err = eval.TestTransactionGroup(txgroup) | ||
| require.Error(t, err) | ||
| require.ErrorContains(t, err, `transaction already in ledger`) |
Contributor
There was a problem hiding this comment.
Definitely not the intent. This is a bad test that is trying to test signature problems with TestTransactionGroup, which doesn't check signatures.
(The test above is also misleading, because the signature is not checked)
cce
force-pushed
the
assert-errorcontains
branch
from
a511472 to
895fc05
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
WIP