-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Fix hardhat-ethers-chai-matchers
#7993
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
Changes from all commits
9f1760f
b2be122
f9b3f04
683b3bd
92284c5
33a3b44
34bdc97
1f8b88a
692a28f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@nomicfoundation/hardhat-ethers-chai-matchers": patch | ||
| --- | ||
|
|
||
| Only use `AssertionError`s within assertion functions [#7993](https://github.com/NomicFoundation/hardhat/pull/7993) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| "@nomicfoundation/hardhat-mocha": patch | ||
| --- | ||
|
|
||
| Improve the error message shown when an `await` is missing [#7993](https://github.com/NomicFoundation/hardhat/pull/7993) |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,8 +6,7 @@ import type { Transaction } from "ethers/transaction"; | |
|
|
||
| import util from "node:util"; | ||
|
|
||
| import { HardhatError } from "@nomicfoundation/hardhat-errors"; | ||
| import { AssertionError } from "chai"; | ||
| import { assert as chaiAssert, AssertionError } from "chai"; | ||
|
|
||
| import { ASSERTION_ABORTED, EMIT_MATCHER } from "../constants.js"; | ||
| import { assertArgsArraysEqual, assertIsNotNull } from "../utils/asserts.js"; | ||
|
|
@@ -30,10 +29,7 @@ async function waitForPendingTransaction( | |
| } | ||
|
|
||
| if (hash === null) { | ||
| throw new HardhatError( | ||
| HardhatError.ERRORS.CHAI_MATCHERS.GENERAL.INVALID_TRANSACTION, | ||
| { transaction: JSON.stringify(tx) }, | ||
| ); | ||
| chaiAssert.fail(`"${JSON.stringify(tx)}" is not a valid transaction`); | ||
| } | ||
|
|
||
| return provider.getTransactionReceipt(hash); | ||
|
|
@@ -74,8 +70,12 @@ export function supportEmit( | |
| } catch (e) { | ||
| if (e instanceof TypeError) { | ||
| const errorMessage = e.message.split(" (argument=")[0]; | ||
| // eslint-disable-next-line no-restricted-syntax -- keep the original chai error structure | ||
| throw new AssertionError(errorMessage); | ||
| const error = new AssertionError(errorMessage); | ||
| /* eslint-disable-next-line @typescript-eslint/consistent-type-assertions | ||
| -- This cast is here because otherwise the CI job that ensures that | ||
| this package still works with chai v5 will fail */ | ||
| (error as any).cause = e; | ||
| throw error; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -89,14 +89,12 @@ export function supportEmit( | |
| const topic = eventFragment.topicHash; | ||
| const contractAddress = contract.target; | ||
| if (typeof contractAddress !== "string") { | ||
| throw new HardhatError( | ||
| HardhatError.ERRORS.CHAI_MATCHERS.GENERAL.CONTRACT_TARGET_MUST_BE_A_STRING, | ||
| ); | ||
| chaiAssert.fail("The contract target should be a string"); | ||
| } | ||
|
|
||
| if (args.length > 0) { | ||
| throw new HardhatError( | ||
| HardhatError.ERRORS.CHAI_MATCHERS.GENERAL.EMIT_EXPECTS_TWO_ARGUMENTS, | ||
| chaiAssert.fail( | ||
| "The .emit matcher expects two arguments: the contract and the event name. Arguments should be asserted with the .withArgs helper.", | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -124,14 +122,15 @@ export function supportEmit( | |
| } | ||
|
|
||
| if (contract.runner === null || contract.runner.provider === null) { | ||
| throw new HardhatError( | ||
| HardhatError.ERRORS.CHAI_MATCHERS.GENERAL.CONTRACT_RUNNER_PROVIDER_NOT_NULL, | ||
| ); | ||
| chaiAssert.fail("contract.runner.provider shouldn't be null"); | ||
| } | ||
|
|
||
| return waitForPendingTransaction(tx, contract.runner.provider).then( | ||
| (receipt) => { | ||
| assertIsNotNull(receipt, "receipt"); | ||
| assertIsNotNull( | ||
| receipt, | ||
| "Transaction's receipt cannot be fetched from the network", | ||
| ); | ||
|
Comment on lines
+130
to
+133
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This makes sense if we assume chai matchers can never be used against a non-automining chain, are we happy with that assumption?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I didn't change that assumption here. It was always present. I guess we can create an issue to revisit it if needed. @ChristopherDedominici Note that these matchers may assume a few extra things that are only present in dev networks.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created this PR (still in draft), to address the issue: #7997
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ChristopherDedominici how does this work in Hardhat 2, did chai matchers always assume you were running against an automine network?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, in HH2 it works like that. Much of this code was inherited from Waffle, which worked only with Ganache.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's not block this PR on the other one though |
||
| return onSuccess(receipt); | ||
| }, | ||
| ); | ||
|
|
@@ -183,7 +182,7 @@ const tryAssertArgsArraysEqual = ( | |
| const parsedLog = chaiUtils | ||
| .flag(context, "contract") | ||
| .interface.parseLog(logs[0]); | ||
| assertIsNotNull(parsedLog, "parsedLog"); | ||
| assertIsNotNull(parsedLog, "Can't parse the first log"); | ||
|
|
||
| return assertArgsArraysEqual( | ||
| Assertion, | ||
|
|
@@ -204,7 +203,7 @@ const tryAssertArgsArraysEqual = ( | |
| const parsedLog = chaiUtils | ||
| .flag(context, "contract") | ||
| .interface.parseLog(logs[index]); | ||
| assertIsNotNull(parsedLog, "parsedLog"); | ||
| assertIsNotNull(parsedLog, `Can't parse the log index ${index}`); | ||
|
|
||
| assertArgsArraysEqual( | ||
| Assertion, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.