refactor(test-forks): Remove block_number/timestamp from all methods#2442
Conversation
df2e91f to
db2570d
Compare
|
@spencer-tb Added automatic ruleset generation, commit 3499a4a contains a unit test that can be executed with |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## forks/amsterdam #2442 +/- ##
================================================
Coverage 86.35% 86.35%
================================================
Files 599 599
Lines 36904 36904
Branches 3771 3771
================================================
Hits 31868 31868
Misses 4485 4485
Partials 551 551
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
241d347 to
47715d8
Compare
|
@fselmo I've fixed the comments and rebased, it's ready for another pass :) |
47715d8 to
c137ac7
Compare
|
@marioevz sorry I took too long here and it needed a rebase again. I see you're working on it now. I will do another pass here when CI stabilizes. Still some conflicts. |
c137ac7 to
e7eb0d4
Compare
|
@fselmo Finally got around to rebase and fix the conflicts, I think this is now ready :) |
fselmo
left a comment
There was a problem hiding this comment.
Only found some remaining doc changes and minor docstring updates. Logic seems sound and lgtm checking the ruleset after the changes here. Going to approve but let's try to make these updates before merging. I'm not sure why some are not showing the proper diff but I think it's because it's in places where you didn't touch in the PR maybe 🤔.
Anyway, logic lgtm! nice refactor 👍🏼
Co-authored-by: felipe <fselmo2@gmail.com>
Co-authored-by: felipe <fselmo2@gmail.com>
e7eb0d4 to
c43098d
Compare
🗒️ Description
Removes
block_numberandtimestampfrom all fork methods. In its place, aTransitionForkis now introduced which can contain two forks (perhaps more in the future) and has to be resolved before accessing any of the forks methods.This forces the tester to resolve the fork before requesting properties from it, instead of always passing
block_number=0, timestamp=0.The affected tests are the ones that use the
valid_at_transition_tomarker, since they now have to declare the fork parameter asfork: TransitionForkinstead of the usualfork: Fork.🔗 Related Issues or PRs
N/A.
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx tox -e statictype(scope):.mkdocs servelocally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.@ported_frommarker.Cute Animal Picture