OpenZeppelin Test Helpers expectRevert.unspecified creates issues when running coverage

Support
1

so we use expect revert.
and sometimes we use expectRevert.unspecified. when we don’t issue any message.

Seems coverage EVM is older. and has different output for revert messages without a string attached.
so for some reason revert.unspecified is affected.
“invalid opcode” is returned.
not what expectRevert.unspecified is catching.

1 Like
Does OpenZeppelin Test Environment make tests faster to write?
2

Hi @ilanD,

What tool are you using for coverage?
Are you able to share a small example of the issue?

3

from NPM
“solidity-coverage”: “0.7.0-beta.3”

can share later example

1 Like
4

so can;t create example right now.
this commit fixes this issue for now by moving to try, catch

should take this code and run with expect revert version. see the error message when running with coverage.

1 Like
5

Hi @ilanD,

I tried to reproduce with solidity-coverage@beta (0.7.0-beta.3) and solidity-coverage (0.7.4) using truffle and no apparent issues.

Coverage

$ npx truffle run coverage

> Using Truffle library from local node_modules.

> server:            http://127.0.0.1:8555
> truffle:           v5.1.22
> ganache-core:      v2.10.2
> solidity-coverage: v0.7.4

Network Info
============
> id:      *
> port:    8555
> network: soliditycoverage


Instrumenting for coverage...
=============================

> Box.sol

Coverage skipped for:
=====================

> Migrations.sol

Compiling your contracts...
===========================
> Compiling ./.coverage_contracts/Box.sol
> Compiling ./.coverage_contracts/Migrations.sol
> Artifacts written to /home/abcoathup/projects/forum/iland/.coverage_artifacts/contracts
> Compiled successfully using:
   - solc: 0.5.16+commit.9c3226ce.Emscripten.clang


Compiling your contracts...
===========================
> Everything is up to date, there is nothing to compile.



  Contract: Box
    ✓ retrieve returns a value previously stored (85ms)
    ✓ reverts when storing a zero value (54ms)


  2 passing (299ms)

------------|----------|----------|----------|----------|----------------|
File        |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
------------|----------|----------|----------|----------|----------------|
 contracts/ |      100 |      100 |      100 |      100 |                |
  Box.sol   |      100 |      100 |      100 |      100 |                |
------------|----------|----------|----------|----------|----------------|
All files   |      100 |      100 |      100 |      100 |                |
------------|----------|----------|----------|----------|----------------|

> Istanbul reports written to ./coverage/ and ./coverage.json
> solidity-coverage cleaning up, shutting down ganache server

Box.sol

// contracts/Box.sol
pragma solidity ^0.5.0;

contract Box {
    uint256 private value;

    // Emitted when the stored value changes
    event ValueChanged(uint256 newValue);

    // Stores a new value in the contract
    function store(uint256 newValue) public {
        require(newValue > 0);
        value = newValue;
        emit ValueChanged(newValue);
    }

    // Reads the last stored value
    function retrieve() public view returns (uint256) {
        return value;
    }
}

Box.test.js

const Box = artifacts.require("Box");

const { BN, expectEvent, expectRevert } = require('@openzeppelin/test-helpers');

contract("Box", async accounts => {

    beforeEach(async function () {
        // Deploy a new Box contract for each test
        this.box = await Box.new();
    });

    it('retrieve returns a value previously stored', async function () {
        const value = new BN('42');
        
        // Store a value - recall that only the owner account can do this!
        const receipt = await this.box.store(value);

        // Test that a ValueChanged event was emitted with the new value
        expectEvent(receipt, 'ValueChanged', { newValue: value });

        // Test if the returned value is the same one
        // Note that we need to use strings to compare the 256 bit integers
        expect((await this.box.retrieve()).toString()).to.equal('42');
    });

    it('reverts when storing a zero value', async function () {
        const value = new BN('0');
        
        // Store a value - recall that only the owner account can do this!
        await expectRevert.unspecified(this.box.store(value));
    });
});
6

WE are using buidler EVM
seems return code is not compatible with ganache for this revert.

1 Like