fix(fill): re-anchor start_block under --no-reset-between-tests - #3507
fix(fill): re-anchor start_block under --no-reset-between-tests#3507jochem-brouwer wants to merge 8 commits into
--no-reset-between-tests#3507Conversation
…tween-tests flag is activated
The session captures start_block once, at the head that follows the
global setup. `_reset_chain_between_tests` rewinds to that block after
every test, which keeps the anchor correct for the next fill.
`--no-reset-between-tests` skips the rewind, but nothing advances the
anchor in its place. The first test moves the head, and every later test
still chains its first block from the stale anchor. The client then
rejects the parent ("parentHash is not current head"), or, on a long run
against a non-archive client, reports "historical state ... is not
available" once that block's state is pruned.
Read the head before each test and re-anchor on it. Consumers are not
affected: they route pre-run setup by directory (`pre_run/*.json`,
applied once per session), not by a fixture's `start_block_hash`.
|
This builds on top of #3304 so that should first be merged. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #3507 +/- ##
===================================================
+ Coverage 93.54% 93.90% +0.35%
===================================================
Files 624 624
Lines 37074 36905 -169
Branches 3394 3326 -68
===================================================
- Hits 34681 34655 -26
+ Misses 1644 1569 -75
+ Partials 749 681 -68
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What do you think about adding that to the top of the PR description and moving this to draft? |
|
@danceratopz that PR (#3304) adds the I have drafted this PR because from https://github.com/jochem-brouwer/execution-specs/pull/6/commits we need to add two more commits 3d072ce and e269bb4. The latter somehow raises a merge conflict if I cherry-pick it. Will look into it. |
Description
Problem
--no-reset-between-testskeeps the chain moving between fills, but thesession anchor never moves with it.
_session_pre_runcapturesstart_blockonce, fromlatest, after theglobal setup:
That is the only production write to
client_backend.start_block. Everytest then chains its first block from it, in
make_stateful_fixture.Normally the invariant holds because
_reset_chain_between_testsrewindsthe chain back to that block after each test. Under
--no-reset-between-teststhe rewind is skipped and nothing takes itsplace, so the anchor goes stale as soon as the first test builds a block.
The nonce side of the flag was already adapted —
worker_keyreads atlatestunder the flag, with a comment explaining the inversion. Theanchor side was missed.
Related Issues or PRs
Stacked on #3304, which introduces the
--no-reset-between-testsflag this fixes. Until that PR merges, the diff here includes its commits; the change in this PR is the single commit on top, by @skylenet. See also jochem-brouwer#6Checklist
just static<type>(<area>): <title>, where<type>and<area>come from an appropriateC-<type>, respectivelyA-<area>, label. The title should match the target squash commit message.Cute Animal Picture