fix: remove message:null sentinel and pin exact Nethermind error messages#24
Draft
manusw7 wants to merge 1 commit into
Draft
fix: remove message:null sentinel and pin exact Nethermind error messages#24manusw7 wants to merge 1 commit into
manusw7 wants to merge 1 commit into
Conversation
…ages Revert the per-fixture "message": null sentinel added in PR #10 and pin the specific error.code + error.message that Nethermind will return once NethermindEth/nethermind#12003 is merged. Why sentinel removal: null-message matching silently passes any message text, which prevents version-to-version regression detection in Nethermind. When a message changes across NM versions the test should fail so we can review the delta intentionally. Why pinned messages: The 13 RLP-decode fixtures (test_01-06, 12-16, 20, 21) currently fail against Nethermind because NM emits -32000/"Invalid RLP." rather than -32602. They will flip to PASS once #12003 lands (which maps catch(RlpException e) to InvalidParams with "Invalid RLP: {e.Message}"). The 10 JSON-RPC param-binding fixtures (test_07-11, 17-19, 22, 23) already pass against NM at -32602; pinning their messages closes the regression gap. Messages captured by running the NM #12003 test suite against each input.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What
\"message\": nullper-fixture sentinel added in feat(compare): per-fixture"message": nullsentinel for code-only error matching #10 (removescomparator.goblock + its two tests).error.code+error.messagein all 23eth_sendRawTransactionfixtures from feat(eth_sendRawTransaction): expand with parameter-validation coverage #11.Why the sentinel had to go
The
\"message\": nullsentinel ignores error message text entirely, which kills version-to-version regression testing for Nethermind. If NM's error wording changes between releases the test still passes silently — we want it to fail so the delta is reviewed intentionally.Why this PR is a draft
The 13 RLP-decode fixtures (
test_01-06, 12-16, 20, 21) currently fail against Nethermind because NM emits-32000 / \"Invalid RLP.\"instead of-32602 / \"Invalid RLP: {specific}\".They will flip to PASS once NethermindEth/nethermind#12003 is merged.
That PR is itself a draft, waiting for go-ethereum to first align in ethereum/go-ethereum#35129.
The merge order is: geth #35129 → NM #12003 → this PR promoted to ready.
Related
"message": nullsentinel for code-only error matching #10