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 shouldFail.reverting.withMessage on non-Ganache #25

Conversation

scottt
Copy link
Contributor

@scottt scottt commented Apr 3, 2019

Currently shouldFail.reverting.withMessage tries to execute the following
regular expression match:

/TestRPC\/v([0-9.]+)\/ethereum-js/.exec(web3ClientVersion)

When web3ClientVersion has a value like
Geth/v1.8.9-unstable-d4ac250e/linux-amd64/go1.9.4,
which would likely occur when testing against any non-Ganache
chain implementation, RegExp.prototype.exec() would return null
causing an error like:

TypeError: Cannot use 'in' operator to search for '1' in null

when 1 in matches is evaluated in the subsequent code.

This patch fixes things by testing for matches being null or
matches[1] not being valid explicitly.

Tested with npm run test and npm run test-integration.

Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

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

Great PR @scottt, everything looks just fine! Thank you very much for your contribution and thorough description!

Could you add an entry to the changelog, under 0.4.0 (unreleased)? Thanks!

Currently `shouldFail.reverting.withMessage` tries to execute the following
regular expression match:
```
/TestRPC\/v([0-9.]+)\/ethereum-js/.exec(web3ClientVersion)
```

When `web3ClientVersion` has a value like
`Geth/v1.8.9-unstable-d4ac250e/linux-amd64/go1.9.4`,
which would likely occur when testing against any non-Ganache
chain implementation, `RegExp.prototype.exec()` would return `null`
causing an error like:
```
TypeError: Cannot use 'in' operator to search for '1' in null
```
when `1 in matches` is evaluated in the subsequent code.

This patch fixes things by testing for `matches` being `null` or
`matches[1]` not being valid explicitly.

Tested with `npm run test` and `npm run test-integration`.
@scottt scottt force-pushed the fix-shouldFail.reverting.withMessage-on-non-ganache branch from 9737be0 to a58fd87 Compare April 3, 2019 17:55
@scottt
Copy link
Contributor Author

scottt commented Apr 3, 2019

Could you add an entry to the changelog, under 0.4.0 (unreleased)? Thanks!

Like this? :)
a58fd87#diff-4ac32a78649ca5bdd8e0ba38b7006a1e

Copy link
Contributor

@nventuro nventuro left a comment

Choose a reason for hiding this comment

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

Fantastic, thanks a bunch!

@nventuro nventuro merged commit 5d25f8e into OpenZeppelin:master Apr 3, 2019
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.

None yet

2 participants