Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce forked upgrade testing #13323

Merged
merged 105 commits into from
Dec 19, 2024
Merged

Introduce forked upgrade testing #13323

merged 105 commits into from
Dec 19, 2024

Conversation

maurelian
Copy link
Contributor

@maurelian maurelian commented Dec 9, 2024

This PR enable running the foundry test suite against a fork of mainnet or sepolia.

Key changes:

  1. A new Upgrade.s.sol which can be etched in place of Deploy.s.sol, and instead of deploying new contracts, it retrieves them from the forked node, and saves them to the deployment artifact so that they will be tested.
  2. Setup.sol has new functionality to enable fork testing:
    • a new state var isForkTest and function skipIfForkTest() these can be used to identify when in a fork, and skip the test or take other action if necessary.
    • l2Genesis.setUp() is skipped and Setup.L2() returns early when in a fork test.
  3. A new just test-upgrade command which sets the env vars necessary to run the test on a fork. This command runs only tests in the test/dispute and test/L1 directory. This is necessary in order to keep the run time reasonable, and the majority of other tests are not relevant to forked upgrade testing.

Limitations:

  1. At this time, many tests are still failing: Encountered a total of 56 failing tests, 164 tests succeeded.
  • While I am working through them with good momentum, I estimate that there are 5 to 20 tweaks still needed to get them working, either by skipping them (with justifications for doing so), or making fixes to the setup, cheats and assertions.
  1. An unavoidable challenge is that it will become more difficult to write new tests as they will need to run against both locally deployed and mainnet forked systems. I believe that is a cost we have to bear, and that overtime we will become better at doing so.
  2. The upgrade path itself has not yet been written, so some tests should be expected to fail for now. In the case where the failure is due to a difference between the system on mainnet vs. develop, they will be skipped, and labelled with TODO(opcm upgrades): so that they can be fixed in a child PR.

@maurelian maurelian requested review from a team as code owners December 9, 2024 20:14
Copy link

codecov bot commented Dec 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 47.26%. Comparing base (b6cf8ee) to head (27263ed).
Report is 4 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #13323      +/-   ##
===========================================
- Coverage    47.59%   47.26%   -0.34%     
===========================================
  Files          943      943              
  Lines        78827    78827              
  Branches       803      803              
===========================================
- Hits         37515    37254     -261     
- Misses       38509    38820     +311     
+ Partials      2803     2753      -50     
Flag Coverage Δ
cannon-go-tests-32 62.27% <ø> (-1.98%) ⬇️
cannon-go-tests-64 57.35% <ø> (-1.62%) ⬇️
contracts-bedrock-tests 89.28% <ø> (-2.70%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 40 files with indirect coverage changes

@maurelian maurelian force-pushed the upgrade-against-registry branch from 49dfc33 to 180b5bc Compare December 9, 2024 20:54
@maurelian maurelian requested a review from a team as a code owner December 9, 2024 21:05
@maurelian maurelian force-pushed the upgrade-against-registry branch from 7cedb35 to 6feb42f Compare December 9, 2024 21:15
@maurelian maurelian added this pull request to the merge queue Dec 19, 2024
Merged via the queue into develop with commit fa60a28 Dec 19, 2024
44 checks passed
@maurelian maurelian deleted the upgrade-against-registry branch December 19, 2024 14:19
@@ -1195,7 +1256,7 @@ workflows:
- contracts-bedrock-tests:
# Test everything except PreimageOracle.t.sol since it's slow.
name: contracts-bedrock-tests
test_list: find test -name "*.t.sol" -not -name "PreimageOracle.t.sol"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I introduced a bug here which caused ONLY the preimage oracle tests to run.

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.

6 participants