-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: Single integration test for deploy campaign #77
Conversation
…nsactions on crucial flows
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feat/deploy-campaign #77 +/- ##
=====================================================
Coverage 99.80% 99.80%
=====================================================
Files 11 11
Lines 502 502
Branches 124 124
=====================================================
Hits 501 501
Misses 1 1 |
@@ -45,7 +45,7 @@ | |||
"@nomicfoundation/hardhat-network-helpers": "^1.0.8", | |||
"@nomicfoundation/hardhat-toolbox": "2.0.2", | |||
"@nomiclabs/hardhat-ethers": "2.2.2", | |||
"@nomiclabs/hardhat-etherscan": "3.0.0", | |||
"@nomiclabs/hardhat-etherscan": "3.1.7", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't get an error with this? I thought this package is already part of the hardhat-toolbox
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had an error without adding this, and this was the resolution. I couldn't setup a Sepolia network anymore in the HH config without upgrading the package
test/helpers/deploy-helpers.ts
Outdated
) : Promise<void> => { | ||
let index = 0; | ||
|
||
// TODO need to return state for outer calling tests, better way to do this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is this for? still needs to be done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No I resolved that it was more meant to be explanatory. I couldn't get the state of the contracts in the scope of the code that calls registerRootDomainBulk
to reflect the changes made within that function if I used a prototype function like .map()
or .reduce()
, I'm sure there is a way but it wouldn't work for me and I went with this instead. Removed the comment
🎉 This PR is included in version 0.8.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Single integration test for deploy campaign that runs deployment and then also proceeds to run through several crucial flows as part of verification.
For 339