Skip to content

Remove unnecessary contract references in integration-tests#2199

Merged
mslipper merged 2 commits intoethereum-optimism:developfrom
tonykogias:remove-contract-references-in-integration-tests
Mar 3, 2022
Merged

Remove unnecessary contract references in integration-tests#2199
mslipper merged 2 commits intoethereum-optimism:developfrom
tonykogias:remove-contract-references-in-integration-tests

Conversation

@tonykogias
Copy link
Copy Markdown
Contributor

Description
Remove unnecessary contract references in integration-tests

Metadata

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 11, 2022

🦋 Changeset detected

Latest commit: a4198a4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/integration-tests Patch

Not sure what this means? Click here to learn what changesets are.

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

@github-actions github-actions bot added the A-integration Area: integration tests label Feb 11, 2022
@smartcontracts
Copy link
Copy Markdown
Contributor

@tonykogias what a beast, nice work!

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 11, 2022

Codecov Report

Merging #2199 (a4198a4) into develop (d0853b1) will increase coverage by 9.49%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2199      +/-   ##
===========================================
+ Coverage    73.04%   82.54%   +9.49%     
===========================================
  Files           86       60      -26     
  Lines         2846     1914     -932     
  Branches       486      282     -204     
===========================================
- Hits          2079     1580     -499     
+ Misses         767      334     -433     
Flag Coverage Δ
batch-submitter 62.63% <ø> (ø)
contracts 90.88% <ø> (ø)
core-utils 87.80% <ø> (ø)
data-transport-layer ?
sdk ?

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

Impacted Files Coverage Δ
packages/sdk/src/adapters/standard-bridge.ts
packages/sdk/src/utils/coercion.ts
packages/sdk/src/utils/merkle-utils.ts
packages/sdk/src/utils/index.ts
...layer/src/services/l1-ingestion/handlers/errors.ts
packages/sdk/src/interfaces/types.ts
packages/data-transport-layer/src/utils/eth-tx.ts
packages/sdk/src/utils/contracts.ts
packages/sdk/src/interfaces/index.ts
...kages/data-transport-layer/src/utils/validation.ts
... and 16 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 d0853b1...a4198a4. Read the comment docs.

@tonykogias tonykogias marked this pull request as ready for review February 12, 2022 20:46
l2SignerOrProvider: l2Wallet,
l1ChainId: network.chainId,
contracts: {
l1: {
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.

You can actually leave this out, the SDK will pick the contract addresses automatically based on the chain ID.

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.

cc @mslipper I just need the contract addresses for nightly so that it will work correctly in the integration tests.

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.

@tonykogias let's remove the entire contracts object here, should be pulled in automatically

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.

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.

cc @mslipper I just need the contract addresses for nightly so that it will work correctly in the integration tests.

We should be able to support addresses that aren't hardcoded - the point of nightly is that it can be blown away and fully redeployed at any moment

Copy link
Copy Markdown
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

Just needs a changeset and this will be good to go. Great work.

@mslipper mslipper changed the title Remove unnecessary contract references in integration-tets Remove unnecessary contract references in integration-tests Mar 2, 2022
@mslipper
Copy link
Copy Markdown
Contributor

mslipper commented Mar 3, 2022

Approved + merging since it has the changeset now.

@mslipper mslipper merged commit 5e0a328 into ethereum-optimism:develop Mar 3, 2022
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.

Remove unnecessary contract references in integration tests

5 participants