TrueFiEng / Waffle

Library for writing and testing smart contracts.
https://getwaffle.io
MIT License
960 stars 161 forks source link

Inconsistent behaviour in recent library upgrade. #825

Open hickscorp opened 1 year ago

hickscorp commented 1 year ago

We recently upgraded our dependencies like follows:

    "@nomiclabs/hardhat-ethers": "^2.2.1",
    "@nomiclabs/hardhat-waffle": "^2.0.0",
    "@typechain/hardhat": "^6.1.4",

to

    "@nomiclabs/hardhat-ethers": "^2.2.2",
    "@nomiclabs/hardhat-waffle": "^2.0.5",
    "@typechain/hardhat": "^6.1.5",

We use custom errors extensively in our Solidity code. For example:

...
revert ICustomErrors.RequiresMarketplaceActiveMembership(params.governor);
...

Before, we were able to match this nicely in our tests, such as:

await expect(subject).to.be
  .revertedWith(`RequiresMarketplaceActiveMembership("${governor.address}")`);

This was particularly useful as some methods might revert with the same custom error (in this example RequiresMarketplaceActiveMembership but with different parameters, based on what went wrong), and we were able to be very thorough in our tests, making them very resilient and not just "coverage for the sake of it".

It seems that someone thought it could be a good idea to strip anything in parentheses of revert messages, so now we can only match such as:

await expect(subject).to.be
  .revertedWith('RequiresMarketplaceActiveMembership');
yivlad commented 1 year ago

If you're using hardhat you can check parameters using withArgs matcher. https://github.com/TrueFiEng/Waffle/blob/0915e7274093f2a1fb1b2a8be0521470a9f8c2f4/waffle-hardhat/test/reverted.test.ts#L62-L69