Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

Fixes false positives in expectTransactionFailedAsync #1852

Merged
merged 1 commit into from
Jun 5, 2019
Merged

Fixes false positives in expectTransactionFailedAsync #1852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions contracts/test-utils/CHANGELOG.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
[
{
"version": "3.1.8",
"changes": [
{
"note": "Fixed false positives in `expectTransactionFailedAsync` and `expectContractCallFailedAsync`",
"pr": 1852
}
]
},
{
"timestamp": 1558712885,
"version": "3.1.7",
Expand Down
6 changes: 4 additions & 2 deletions contracts/test-utils/src/assertions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ export async function expectTransactionFailedAsync(p: sendTransactionResult, rea
}
switch (nodeType) {
case NodeType.Ganache:
return expect(p).to.be.rejectedWith(reason);
const rejectionMessageRegex = new RegExp(`^VM Exception while processing transaction: revert ${reason}$`);
return expect(p).to.be.rejectedWith(rejectionMessageRegex);
case NodeType.Geth:
logUtils.warn(
'WARNING: Geth does not support revert reasons for sendTransaction. This test will pass if the transaction fails for any reason.',
Expand Down Expand Up @@ -160,7 +161,8 @@ export async function expectTransactionFailedWithoutReasonAsync(p: sendTransacti
* otherwise resolve with no value.
*/
export async function expectContractCallFailedAsync<T>(p: Promise<T>, reason: RevertReason): Promise<void> {
return expect(p).to.be.rejectedWith(reason);
const rejectionMessageRegex = new RegExp(`^VM Exception while processing transaction: revert ${reason}$`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this will work in Geth. We have disabled Geth tests for now (see #1754 and #1775). Going forward, instead of running all of our unit tests through Geth and relying on features that don't exist upstream, we want to have integration tests that operate at a higher level and don't rely on any extra features.

Anyways, I'm okay with this for now but if we write integration tests for Geth in the future we'll have to update it.

return expect(p).to.be.rejectedWith(rejectionMessageRegex);
}

/**
Expand Down