Skip to content

Conversation

pcarranzav
Copy link
Member

Since #517, our hardhat local network mines blocks in two ways:

  • Automatically when any transaction is sent
  • Every 30 seconds

I suspect this is breaking our unit tests for a few reasons:

  • CI has started failing quite frequently since that commit
  • Several of the failed tests look like timing issues
  • I manually timed running tests and noticed errors tend to happen close to 30-second boundaries (though this was a bit of a handwavy measurement).

This PR changes things in two ways:

  • For the default local network, disable automining, so transactions will not trigger a mined block, and instead they will only be mined every 30 seconds (@fordN could you confirm if this is the desired behavior?)
  • For the unit tests, enable automining and disable the mining interval.

I've run yarn test several times in this branch and haven't seen it fail so far (whereas it often fails if I run it from the main dev branch).

An additional small change: we allow passing file paths to yarn test so that we can optionally run only specific testfiles.

@pcarranzav pcarranzav requested review from abarmat and fordN March 28, 2022 19:46
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #554 (5faa832) into dev (3538e00) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #554   +/-   ##
=======================================
  Coverage   90.58%   90.58%           
=======================================
  Files          37       37           
  Lines        1795     1795           
  Branches      293      293           
=======================================
  Hits         1626     1626           
  Misses        169      169           
Flag Coverage Δ
unittests 90.58% <ø> (ø)

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 87a2468...5faa832. Read the comment docs.

Copy link
Contributor

@abarmat abarmat left a comment

Choose a reason for hiding this comment

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

Good find! I'm good with it, only thing would be to check if Ford needs automine by default

Copy link
Contributor

@fordN fordN left a comment

Choose a reason for hiding this comment

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

Works for me! The current setup with both automining and timed mining was somewhat arbitrary.

@pcarranzav pcarranzav merged commit 2f92d5f into dev Mar 30, 2022
@pcarranzav pcarranzav deleted the pcv/fix-tests branch March 30, 2022 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants