Run tests twice

[kanej]

This is a mocha caching and cleanup issue addressed here: mochajs/mocha#2783.

Relates to #1720

Testing

An integration test has been added to e2e to cover the run twice scenario and guard against regression of the bug.

Manual

To test manually:

1. create a new advanced typescript hardhat project
2. link to `hardhat-core
3. create a new scripts file ./scripts/multi-run.js:

// eslint-disable-next-line import/no-extraneous-dependencies
const hre = require("hardhat");

async function main() {
  const code = await hre.run("test");

  if (code > 0) {
    console.error("Failed first test run");
    process.exit(1);
  }

  const secondCode = await hre.run("test");

  if (secondCode > 0) {
    console.error("Failed second test run");
    process.exit(1);
  }

  console.log("\nFinished and complete\n");
}

main()
  .then(() => process.exit(0))
  .catch((error) => {
    console.error(error);
    process.exit(1);
  });

4. Run the script npx hardhat run ./scripts/multi-run.js
5. Confirm the script runs without error and that tests are present in both runs:

As a test against a more in-depth test suite, I did the equivalent of the above on the open-zepplin test suite (updating to the latest hardhat libs first) and was able to run the suite twice programmatically with the script above.

TODO

• Apply fix from mocha suggestions
• Look at options on unit tests
• Run against existing project with large test suite

[changeset-bot]

🦋 Changeset detected

Latest commit: 80a4437

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

This PR includes changesets to release 1 package

Name	Type
hardhat	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

[alcuadrado]

This is really interesting! This new API of mocha is something I wasn't expecting tbh.

I think the decision we need to make is if we are ok with bumping the mocha dependency. If we do, I think we should:

1. Document that we may do it in our stability guarantees docs.
2. Bump the minor version.
3. Make sure that changes between 5 and 9 aren't too aggressive.
4. Make sure mocha 9 supports all the node.js versions than we do.

[kanej]

This is really interesting! This new API of mocha is something I wasn't expecting tbh.

I think the decision we need to make is if we are ok with bumping the mocha dependency. If we do, I think we should:

1. Document that we may do it in our stability guarantees docs.

2. Bump the minor version.

3. Make sure that changes between 5 and 9 aren't too aggressive.

4. Make sure mocha 9 supports all the node.js versions than we do.

This fix is adding mocha.dispose after run in the test task. This resolves the problem in local testing for a simple project - more testing is needed on larger test suites.

dispose was introduced in mocha 7.2.0 changelog as part of Mocha's multi-run fix (mochajs/mocha#4234).

Hardhat-core has a dependency on ^7.1.2 which get resolved to 7.2.0 - so dispose is present in a local build. dispose is not present in our current @types/mocha however, and you have to bump the version to 9 to get it.

So in this PR I propose:

• bump mocha as a dep in hardhat-core to 7.2.0 to be explicit
• bump mocha as a dev-dep everywhere else to 7.2.0 for consistency in our local dev environment
• bump @types/mocha to 9 in dev-deps in all packages to allow use of dispose and consistency across packages - no code changes are required to support this. Note that the sample typescript project already provides a default @types/mocha on version 9.0.0

I think moving the dep in hardhat-core from ^7.1.2 to 7.2.0 is safe and explicit and doesn't require a change to our stability promises. From the package.json mocha 7.2 supports node 8.10 and above:

@alcuadrado @fvictorio

[alcuadrado]

That's a good find! I think that makes sense

[kanej]

I have pulled the open-zepplin repo and done a multi-test script using this branch, and the their suite ran twice successfully with all tests.

If we are happy with the version bumps this PR is ready for review.

[fvictorio]

Just one very minor comment, otherwise LGTM