Skip to content

testgen: fix duplicate test names and regenerate fixtures - #858

Merged
lightclient merged 2 commits into
ethereum:mainfrom
MysticRyuujin:testgen-fix-duplicate-test-names
Aug 11, 2026
Merged

lightclient merged 2 commits into
ethereum:mainfrom
MysticRyuujin:testgen-fix-duplicate-test-names

Conversation

@MysticRyuujin

Copy link
Copy Markdown
Contributor

Summary

Supersedes #850, which found the bug. That PR renames one colliding eth_simulateV1
test but does not regenerate the fixtures, so CI fails at "Fail if any files are
untracked or have changes"
. This PR fixes all three collisions in the repo,
regenerates the fixtures, and adds a guard so the class cannot recur silently.

rpctestgen writes each fixture to tests/<method>/<test>.io and never checks for a
repeated name (tools/cmd/rpctestgen/generate.go), so two tests of the same method
sharing a name silently overwrite each other — last one wins.

The three collisions

Method Name Nature
eth_simulateV1 ethSimulate-simple-state-diff two distinct tests
eth_simulateV1 ethSimulate-simple-more-params-validate byte-identical copies
eth_getLogs filter-with-blockHash copies differing only by a temp variable

ethSimulate-simple-state-diff — real coverage was lost

The two tests exercise different things: one uses a stateDiff override (patches
individual slots), the other a full state override (replaces all storage). Because
the second won the write, the committed fixture held only the full-state case — the
stateDiff case had never been recorded.

Renaming the full-override case to ethSimulate-simple-state restores both. Reading
storage slot 1 in the second block now shows the distinction:

fixture slot 1 why
ethSimulate-simple-state-diff.io 0x…02 preserved, only slot 0 patched
ethSimulate-simple-state.io 0x…00 wiped, all storage replaced

Also drops a stale copy-paste comment that labelled the State field a "state diff
override".

The other two — pure duplicates

Both duplicates produce an identical request/response exchange, so deleting them
loses no coverage. Proof: after deletion the first case of each pair writes the
fixture instead of the second, and tests/eth_getLogs/filter-with-blockHash.io and
tests/eth_simulateV1/ethSimulate-simple-more-params-validate.io come out
byte-unchanged.

If a distinct second case was ever intended in either spot, it was never actually
written.

The guard

TestUniqueTestNames walks AllMethods and fails on a repeated name within a
method. Verified load-bearing: it reports all three collisions against main and
passes here.

Nothing invoked the tools test suite in CI — lint-tools runs make lint and the
root make test is speccheck only — so this adds a make test step next to the
existing lint step. That also starts running the pre-existing internal/specgen
tests.

Test plan

  • make build + make test (speccheck): all passing.
  • make lint (openrpc-linter): no new warnings
  • cd tools && make lint (gofmt, vet, staticcheck): clean
  • cd tools && make test: passes, including the new guard
  • Full make fill is idempotent — the gate testgen: fix duplicate name for eth_simulateV1 state override test #850 fails is clean here
  • TestUniqueTestNames fails on main with all three collisions, passes here
  • hive rpc-compat/eth_simulateV1 across go-ethereum, nethermind, erigon, besu,
    reth, ethrex — both renamed fixtures pass on geth, nethermind, erigon and reth

hive notes: besu and ethrex fail these two fixtures, both pre-existing and unrelated.
ethrex returns -32601 Method not found for eth_simulateV1 (92 of 93 tests). besu
diverges only on synthetic transaction hashes, across 54 of 93 tests including
fixtures this PR does not touch; its returnData, gasUsed and block fields match.

MysticRyuujin and others added 2 commits August 3, 2026 12:00
Fixtures are written to tests/<method>/<test>.io, so two tests of the same
method sharing a name silently overwrite each other. Three collisions existed:

- eth_simulateV1 "ethSimulate-simple-state-diff" named two distinct tests, one
  using a stateDiff override and one a full state override. The committed
  fixture held only the latter, so the stateDiff case was never recorded.
  Rename the full-override case to "ethSimulate-simple-state".
- eth_getLogs "filter-with-blockHash" and eth_simulateV1
  "ethSimulate-simple-more-params-validate" were each duplicated with no
  behavioural difference. Drop the copies; their fixtures are unchanged.

Co-authored-by: Weixie Cui <cuiweixie@gmail.com>
Nothing ran the tools test suite in CI, so add a step for it alongside the
existing lint job.

@bomanaps bomanaps left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

:shipit: LGTM

@lightclient lightclient left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@lightclient
lightclient merged commit 742d45d into ethereum:main Aug 11, 2026
5 checks passed
@MysticRyuujin
MysticRyuujin deleted the testgen-fix-duplicate-test-names branch August 11, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants