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

Tests: Fix solidity tests #616

Merged
merged 6 commits into from
Aug 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ yarn-error.log
package-lock.json

# coverage
.coverage*
coverage/
coverageEnv/
coverage.json
Expand Down
9 changes: 0 additions & 9 deletions contracts/test/tests/TestDelegateProxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import "../../evmscript/ScriptHelpers.sol";
contract Target {
function dontReturn() public pure {}
function fail() public pure { revert(); }
function die() public { selfdestruct(0); }
}


Expand Down Expand Up @@ -66,12 +65,4 @@ contract TestDelegateProxy is DelegateProxy {
bool result = isContract(nonContract);
Assert.isFalse(result, "should return false");
}

// keep as last test as it will kill this contract
function testDieIfMinReturn0() public {
Assert.isTrue(true, ''); // Make at least one assertion to satisfy the runner

delegatedFwd(target, target.die.selector.toBytes());
Assert.fail('should be dead');
}
}
37 changes: 37 additions & 0 deletions contracts/test/tests/TestDelegateProxySelfDestruct.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
pragma solidity 0.4.24;

import "../helpers/Assert.sol";

import "../../common/DelegateProxy.sol";
import "../../evmscript/ScriptHelpers.sol";


contract Target {
function die() public { selfdestruct(0); }
}

contract TestDelegateProxySelfDestruct is DelegateProxy {
using ScriptHelpers for *;

Target target;

// Mock ERCProxy implementation
function implementation() public view returns (address) {
return this;
}

function proxyType() public pure returns (uint256) {
return FORWARDING;
}

function beforeAll() public {
target = new Target();
}

function testDieIfMinReturn0() public {
Assert.isTrue(true, ''); // Make at least one assertion to satisfy the runner

delegatedFwd(target, target.die.selector.toBytes());
Assert.fail('should be dead');
}
}
7 changes: 3 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@aragon/os",
"version": "5.0.0-beta.8",
"version": "5.0.0-rc.0",
"author": "Aragon Association <[email protected]>",
"license": "(GPL-3.0-or-later OR MIT)",
"repository": "https://github.com/aragon/aragonOS",
Expand All @@ -27,20 +27,19 @@
},
"dependencies": {},
"devDependencies": {
"@aragon/contract-helpers-test": "^0.1.0-rc.3",
"@aragon/contract-helpers-test": "^0.1.0",
"@codechecks/client": "^0.1.5",
"@nomiclabs/buidler": "^1.4.3",
"@nomiclabs/buidler-ganache": "^1.3.3",
"@nomiclabs/buidler-truffle5": "^1.3.4",
"@nomiclabs/buidler-web3": "^1.3.4",
"buidler-extract": "^1.0.0",
"buidler-gas-reporter": "^0.1.3",
"chai": "^4.2.0",
"coveralls": "^3.0.9",
"eth-ens-namehash": "^2.0.8",
"ethereumjs-abi": "^0.6.5",
"solidity-coverage": "^0.7.9",
"solium": "^1.2.5",
"buidler-extract": "^1.0.0",
"web3": "^1.2.11"
}
}
4 changes: 2 additions & 2 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@
<img src="https://img.shields.io/badge/security-audited-green?style=flat-square" alt="Security" />
</a>
<!-- Coverage -->
<a href="https://coveralls.io/github/aragon/aragonOS?branch=master">
<img src="https://img.shields.io/coveralls/aragon/aragonOS/master.svg?style=flat-square" alt="Coverage" />
<a href="https://codecov.io/gh/aragon/aragonOS">
<img src="https://img.shields.io/codecov/c/gh/aragon/aragonOS" />
</a>
</div>

Expand Down
1 change: 1 addition & 0 deletions test/contracts/common/delegate_proxy.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const runSolidityTest = require('../../helpers/runSolidityTest')

runSolidityTest('TestDelegateProxy')
runSolidityTest('TestDelegateProxySelfDestruct')
49 changes: 16 additions & 33 deletions test/helpers/runSolidityTest.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const { decodeEvents } = require('@aragon/contract-helpers-test')
const { getEventAt } = require('@aragon/contract-helpers-test')

const ASSERT_LIB_EVENTS_ABI = [
{
Expand Down Expand Up @@ -27,24 +27,8 @@ const HOOKS_MAP = {
afterAll: 'afterAll',
}

const processResult = (txReceipt, mustAssert) => {
if (!txReceipt || !txReceipt.receipt) {
return
}
const decodedLogs = decodeEvents(txReceipt.receipt, ASSERT_LIB_EVENTS_ABI, 'TestEvent')
decodedLogs.forEach(log => {
if (log.event === 'TestEvent' && log.args.result !== true) {
throw new Error(log.args.message)
}
})
if (mustAssert && !decodedLogs.length) {
throw new Error('No assertions made')
}
}

/*
* Deploy and link `libName` to provided contract artifact.
* Modifies bytecode in place
* Deploy and link `libName` to provided contract artifact
*/
const linkLib = async (contract, libName) => {
const library = await artifacts.require(libName).new()
Expand All @@ -55,34 +39,33 @@ const linkLib = async (contract, libName) => {
* Runs a solidity test file, via javascript.
* Required to smooth over some technical problems in solidity-coverage
*
* @param {string} c Name of solidity test file
* @param {string} testContract Name of solidity test file
*/
function runSolidityTest(c, mochaContext) {
const artifact = artifacts.require(c)
contract(c, accounts => {
function runSolidityTest(testContract, mochaContext) {
const artifact = artifacts.require(testContract)

contract(testContract, () => {
let deployed

before(async () => {
await linkLib(artifact, 'Assert')

deployed = await artifact.new()
})

const assertResult = async call => {
const receipt = await call()
const { args: { result, message } } = getEventAt(receipt, 'TestEvent', { decodeForAbi: ASSERT_LIB_EVENTS_ABI })
if (!result) throw new Error(message || 'No assertions made')
}

mochaContext('> Solidity test', () => {
artifact.abi.forEach(interface => {
if (interface.type === 'function') {
// Set up hooks
if (['beforeAll', 'beforeEach', 'afterEach', 'afterAll'].includes(interface.name)) {
// Set up hooks
global[HOOKS_MAP[interface.name]](async () => {
const receipt = await deployed[interface.name]()
processResult(receipt, false)
})
global[HOOKS_MAP[interface.name]](async () => await deployed[interface.name]())
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, double checking here, does this fail properly if one of the hooks throws?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case some of these reverts, it throws. Note that previously processResult was called with mustAssert set to false always, therefore no assertions were actually being made, we were simply executing calls

} else if (interface.name.startsWith('test')) {
it(interface.name, async () => {
const { tx } = await deployed[interface.name]()
const receipt = await web3.eth.getTransactionReceipt(tx)
processResult(receipt, true)
})
it(interface.name, async () => await assertResult(deployed[interface.name]))
}
}
})
Expand Down
31 changes: 5 additions & 26 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
# yarn lockfile v1


"@aragon/contract-helpers-test@^0.1.0-rc.3":
version "0.1.0-rc.3"
resolved "https://registry.yarnpkg.com/@aragon/contract-helpers-test/-/contract-helpers-test-0.1.0-rc.3.tgz#dec428b5f0f7ffdf851663fd308dbc890ee045c1"
integrity sha512-9bCQagWrrZqaM8pSZq2YAY/uavbuNeaiUnKILFXuVwrVTeX2tjExIcaVTyeiGzZDF/L52ruOMi/o+Ns+T2U/gQ==
"@aragon/contract-helpers-test@^0.1.0":
version "0.1.0"
resolved "https://registry.yarnpkg.com/@aragon/contract-helpers-test/-/contract-helpers-test-0.1.0.tgz#5c5f09739a0b33ab66843bc4849cb2b997d88af0"
integrity sha512-xP2CqqP0Jw/6Jdo1mRg4OxL+3gmsCYV3EJRy7xN8xUrhQIqkOyRptC44X951O7Cr+VeqXJq22rpZSr01TJZhNg==
dependencies:
web3-eth-abi "1.2.5"
web3-utils "1.2.5"
Expand Down Expand Up @@ -2445,17 +2445,6 @@ cors@^2.8.1:
object-assign "^4"
vary "^1"

coveralls@^3.0.9:
version "3.1.0"
resolved "https://registry.yarnpkg.com/coveralls/-/coveralls-3.1.0.tgz#13c754d5e7a2dd8b44fe5269e21ca394fb4d615b"
integrity sha512-sHxOu2ELzW8/NC1UP5XVLbZDzO4S3VxfFye3XYCznopHy02YjNkHcj5bKaVw2O7hVaBdBjEdQGpie4II1mWhuQ==
dependencies:
js-yaml "^3.13.1"
lcov-parse "^1.0.0"
log-driver "^1.2.7"
minimist "^1.2.5"
request "^2.88.2"

create-ecdh@^4.0.0:
version "4.0.4"
resolved "https://registry.yarnpkg.com/create-ecdh/-/create-ecdh-4.0.4.tgz#d6e7f4bffa66736085a0762fd3a632684dabcc4e"
Expand Down Expand Up @@ -5670,11 +5659,6 @@ lcid@^2.0.0:
dependencies:
invert-kv "^2.0.0"

lcov-parse@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/lcov-parse/-/lcov-parse-1.0.0.tgz#eb0d46b54111ebc561acb4c408ef9363bdc8f7e0"
integrity sha1-6w1GtUER68VhrLTECO+TY73I9+A=

lead@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/lead/-/lead-1.0.0.tgz#6f14f99a37be3a9dd784f5495690e5903466ee42"
Expand Down Expand Up @@ -5891,11 +5875,6 @@ lodash@^4.17.4:
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.20.tgz#b44a9b6297bcb698f1c51a3545a2b3b368d59c52"
integrity sha512-PlhdFcillOINfeV7Ni6oF1TAEayyZBoZ8bcshTHqOYJYlrqzRK5hagpagky5o4HfCzzd1TRkXPMFq6cKk9rGmA==

log-driver@^1.2.7:
version "1.2.7"
resolved "https://registry.yarnpkg.com/log-driver/-/log-driver-1.2.7.tgz#63b95021f0702fedfa2c9bb0a24e7797d71871d8"
integrity sha512-U7KCmLdqsGHBLeWqYlFA0V0Sl6P08EE1ZrmA9cxjUE0WVqT9qnyVDPz1kzpFEP0jdJuFnasWIfSd7fsaNXkpbg==

[email protected]:
version "3.0.0"
resolved "https://registry.yarnpkg.com/log-symbols/-/log-symbols-3.0.0.tgz#f3a08516a5dea893336a7dee14d18a1cfdab77c4"
Expand Down Expand Up @@ -7620,7 +7599,7 @@ request-promise@^4.2.2:
stealthy-require "^1.1.1"
tough-cookie "^2.3.3"

request@^2.79.0, request@^2.85.0, request@^2.88.0, request@^2.88.2:
request@^2.79.0, request@^2.85.0, request@^2.88.0:
version "2.88.2"
resolved "https://registry.yarnpkg.com/request/-/request-2.88.2.tgz#d73c918731cb5a87da047e207234146f664d12b3"
integrity sha512-MsvtOrfG9ZcrOwAW+Qi+F6HbD0CWXEh9ou77uOb7FM2WPhwT7smM833PzanhJLsgXjN89Ir6V2PczXNnMpwKhw==
Expand Down