Skip to content

feat(sdk): start using sdk in integration tests#2122

Merged
smartcontracts merged 1 commit intodevelopfrom
sc/sdk-integration-1
Feb 3, 2022
Merged

feat(sdk): start using sdk in integration tests#2122
smartcontracts merged 1 commit intodevelopfrom
sc/sdk-integration-1

Conversation

@smartcontracts
Copy link
Contributor

Description
First PR in a series of PRs to start integrating the SDK into the integration tests. It's too much work to do this all in one PR. Includes a few minor bugfixes to the SDK.

@changeset-bot
Copy link

changeset-bot bot commented Feb 3, 2022

🦋 Changeset detected

Latest commit: edb2184

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 3, 2022
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 removed this test because we should just handle this as part of 1.0. No need to have a dead test here for behavior we haven't specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of making this a second test, we can just assert this inside of the above test which does exactly the same L1 to L2 message. Slightly speeds up the integration tests, which will add up as we start consolidating more tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note to self: maybe worth having sendMessageToL1 and sendMessageToL2 functions to avoid needing the direction parameter

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'm really happy with how simple this is

@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2022

Codecov Report

Merging #2122 (31f0b4f) into develop (fc0b313) will decrease coverage by 0.99%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2122      +/-   ##
===========================================
- Coverage    73.66%   72.66%   -1.00%     
===========================================
  Files           67       84      +17     
  Lines         2187     2883     +696     
  Branches       327      481     +154     
===========================================
+ Hits          1611     2095     +484     
- Misses         576      788     +212     
Flag Coverage Δ
batch-submitter 62.63% <ø> (ø)
contracts 90.48% <ø> (ø)
core-utils 59.94% <ø> (ø)
data-transport-layer 37.74% <ø> (ø)
message-relayer 70.86% <ø> (?)
sdk 69.24% <100.00%> (?)

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

Impacted Files Coverage Δ
packages/sdk/src/utils/contracts.ts 100.00% <ø> (ø)
packages/sdk/src/utils/merkle-utils.ts 27.27% <100.00%> (ø)
packages/sdk/src/interfaces/types.ts 100.00% <0.00%> (ø)
packages/sdk/src/interfaces/index.ts 100.00% <0.00%> (ø)
packages/sdk/src/utils/coercion.ts 90.00% <0.00%> (ø)
packages/sdk/hardhat.config.ts 100.00% <0.00%> (ø)
packages/message-relayer/hardhat.config.ts 100.00% <0.00%> (ø)
packages/sdk/src/cross-chain-messenger.ts 69.02% <0.00%> (ø)
... and 9 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 fc0b313...31f0b4f. Read the comment docs.

@smartcontracts smartcontracts merged commit b93353a into develop Feb 3, 2022
@smartcontracts smartcontracts deleted the sc/sdk-integration-1 branch February 3, 2022 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-integration Area: integration tests A-ops Area: ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants