Coverage ignore large test

[bh2smith]

Based on the solidity-coverage advanced usage guide here

we are able to ignore the tests with the @skip-on-coverage tag included in the test message.

This is directly advised as the proper use from their readme

Coverage distorts gas consumption.
Tests that check exact gas consumption should be skipped.

Note that, in order for the test ignored during coverage to be executed, we have moved it into a separate file and included an additional truffle test for this file. This is not ideal, but does work as expected without executing any tests twice.

[josojo]

I only hope that Travis is also running these tests separately.

I don't think that this is happening, is it? Currently, we are only running coverage tests, as otherwise many tests would be run twice!

[bh2smith]

I don't think that this is happening, is it? Currently, we are only running coverage tests, as otherwise many tests would be run twice!

I am going to try putting the large test into its own file and running this test independently via

yarn run test ./test/stablex/stablex_large_test.js

[nlordell]

Would it be possible to only run tests with the @skip-on-coverage for the second step of the test? That way if we add more tests that should be skipped they automatically get picked up?

[bh2smith]

Would it be possible to only run tests with the @skip-on-coverage for the second step of the test? That way if we add more tests that should be skipped they automatically get picked up?

This is likely possible, but I will have to dig into the solidity-coverage code to see how they do this...

[bh2smith]

@nlordell - I wonder if it would be reasonable to restructure the test folder one for which we run solidity-coverage and the other where we run truffle-test

[nlordell]

I wonder if it would be reasonable to restructure the test folder one for which we run solidity-coverage and the other where we run truffle-test

That could work, don’t know enough about truffle test to tell you if that’s the best way to go though.

[josojo]

Nice... looks real good.

Now, that we are running the test no longer in the coverage mode, we could also reduce the constant

      const sixPointFiveMillion = 6500000

to 5.5 M

[fleupold]

I like the idea of grepping on @solidity-no-coverage to make sure all tests are run in either mode. However, looks like the grep is not yet supported for regular truffle test: trufflesuite/truffle#2080

[bh2smith]

we could also reduce the constant

Will do.

[bh2smith]

I like the idea of grepping on @solidity-no-coverage to make sure all tests are run in either mode. However, looks like the grep is not yet supported for regular truffle test: trufflesuite/truffle#2080

How about we land this for now and create an issue for the future (i.e. when this is available in truffle)

[nlordell]

How about we land this for now and create an issue for the future (i.e. when this is available in truffle)

Sure, but then we should probably remove the mocha options in the truffle-config.js or else when I run truffle test locally, it will not run that test.

[bh2smith]

Sure, but then we should probably remove the mocha options in the truffle-config.js or else when I run truffle test locally, it will not run that test.

Those mocha options are not in the truffle config, they are in the coverage config and they are necessary for our coverage to ignore the gas estimation test. I will update the PR description to summarize the changes introduced here.

[nlordell]

Those mocha options are not in the truffle config, they are in the coverage config and they are necessary for our coverage to ignore the gas estimation test.

Oops! My bad. So running truffle test pulls in these tests?

[bh2smith]

So running truffle test pulls in these tests?

truffle test still executes as usual/expected without any change