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

Fix hardhat_reset (#667) and add unit tests #681

Merged
merged 6 commits into from
Jan 17, 2022
Merged

Conversation

cgewecke
Copy link
Member

@cgewecke cgewecke commented Jan 14, 2022

Co-authored-by: Andres Adjimann [email protected]

Have written a test for #667 but it fails. My expectation is that given a contract with 2 simple functions I can:

  • call function 1
  • hardhat_reset
  • call function 2

... and have 100% line coverage. Instead I see this:

Screen Shot 2022-01-15 at 8 46 26 AM

In the hardhat code it looks like the hardhat node is destroyed and recreated on each reset event. For solidity-coverage to re-attach a step listener to its vm it's important that we get a reference to the new node object. Have found this to be impossible so far.

Event listeners attached to the new (or is it old for some reason?) vm never fire.

Note: I've slightly modified the logic in the new PR to use an env scoped reference to the network provider but the incorrect behavior exists for #667's approach as well. Have also tried:

  • using setTimeout in various places to make sure I'm not running into an event related race condition.
  • re-running nomiclabsUtils.setupHardhatNetwork(env, api, ui) in the reset event call back

Co-authored-by: Andres Adjimann <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Jan 14, 2022

Codecov Report

Merging #681 (a0cc952) into master (cdab28c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #681   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files          22       22           
  Lines         993      996    +3     
=======================================
+ Hits          980      983    +3     
  Misses         13       13           
Impacted Files Coverage Δ
plugins/hardhat.plugin.js 93.68% <100.00%> (+0.20%) ⬆️

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 cdab28c...a0cc952. Read the comment docs.

@cgewecke cgewecke merged commit 5b7eb34 into master Jan 17, 2022
@cgewecke cgewecke deleted the hardhat-reset-tests branch January 17, 2022 14:26
@jummy123
Copy link

Thank you for your work on this fix ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants