Skip to content

Itests mainnet prep#1729

Merged
mslipper merged 1 commit intoethereum-optimism:regenesis/0.5.0from
mslipper:feat/itests-mainnet
Nov 10, 2021
Merged

Itests mainnet prep#1729
mslipper merged 1 commit intoethereum-optimism:regenesis/0.5.0from
mslipper:feat/itests-mainnet

Conversation

@mslipper
Copy link
Copy Markdown
Contributor

@mslipper mslipper commented Nov 9, 2021

This PR gets integration tests into shape to work against mainnet. Specific changes include:

  • Stress tests do not run against mainnet since they are very costly.
  • You can now provide env vars that define addresses for the Uniswap position manager and router. This avoids the need to deploy them ourselves on mainnet.
  • Withdrawal tests are disabled because the withdrawal window on mainnet is 1 week.
  • Mainnet L2 gas prices are pulled from the sequencer rather than using a hardcoded value.
  • The L2 wallet is funded with 1ETH rather than 20. Subsequent tests runs will "top up" the L2 wallet up to 1 ETH.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 9, 2021

⚠️ No Changeset found

Latest commit: d1535df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions github-actions bot added the A-integration Area: integration tests label Nov 9, 2021
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 9, 2021

Codecov Report

Merging #1729 (d1535df) into regenesis/0.5.0 (8282054) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           regenesis/0.5.0    #1729   +/-   ##
================================================
  Coverage            72.64%   72.64%           
================================================
  Files                   69       69           
  Lines                 2274     2274           
  Branches               337      337           
================================================
  Hits                  1652     1652           
  Misses                 622      622           
Flag Coverage Δ
batch-submitter 61.56% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.72% <ø> (ø)
data-transport-layer 38.23% <ø> (ø)
message-relayer 83.17% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8282054...d1535df. Read the comment docs.

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.

Out of scope of this PR but I think defaultTransactionFactory should accept and object where the keys are the values that overrides what it returns by default

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.

What do you think about parsing the env as part of OptimismEnv? And also logging the values in the before block so we know they are being set

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm torn on this one. IMO OptimismEnv should contain stuff related to Optimism itself. Then each test can define its own env vars if they need additional ones. My worry about adding this to OptimismEnv is turning it into a "god class" that's difficult to maintain.

Agreed re: logging - I'll put that in.

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.

I see your point - I wouldn't be opposed to defining another getter class for the uniswap env vars to abstract away process.env, but fine as is also

@mslipper mslipper force-pushed the feat/itests-mainnet branch from 7f91bed to cdfdcc4 Compare November 9, 2021 23:07
@mslipper mslipper force-pushed the feat/itests-mainnet branch from cdfdcc4 to d1535df Compare November 9, 2021 23:12
@mslipper mslipper merged commit 57a0d15 into ethereum-optimism:regenesis/0.5.0 Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-integration Area: integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants