Skip to content

Commit

Permalink
Tests: Refactors and enhancements (#525)
Browse files Browse the repository at this point in the history 
* tests: refactors and enhancements

* helpers: split assert and getter events files
  • Loading branch information
@facuspagnuolo
facuspagnuolo authored May 9, 2019
1 parent 220b6b1 commit d10d195
Show file tree
Hide file tree
Showing 27 changed files with 345 additions and 562 deletions.
20 changes: 8 additions & 12 deletions test/contracts/apm/apm_namehash.js
View file
Original file line number Diff line number Diff line change
@@ -1,32 +1,28 @@
const namehash = require('eth-ens-namehash').hash
const { hash } = require('eth-ens-namehash')
const APMNamehashMock = artifacts.require('APMNamehashMock')

contract('APM Name Hash', accounts => {
contract('APM Name Hash', () => {
let apmNamehashMock

before(async() => {
//console.log("eth: " + namehash('eth'))
//console.log("aragonpm.eth: " + namehash('aragonpm.eth'))
apmNamehashMock = await APMNamehashMock.new()
})

const checkName = async (name) => {
const node = namehash(name + '.aragonpm.eth')
//await apmNamehashMock.getAPMNamehash(name)
const assertNamehash = async (name) => {
const node = hash(name + '.aragonpm.eth')
const apmNamehash = await apmNamehashMock.getAPMNamehash(name)
//console.log("node: " + node)
return apmNamehash.toString() == node
assert.equal(node, apmNamehash.toString(), 'hashes do not match')
}

it('Kernel name hash matches', async () => {
assert.isTrue(await checkName('kernel'), 'hashes should match')
await assertNamehash('kernel')
})

it('ACL name hash matches', async () => {
assert.isTrue(await checkName('acl'), 'hashes should match')
await assertNamehash('acl')
})

it('EVM Registry name hash matches', async () => {
assert.isTrue(await checkName('evmreg'), 'hashes should match')
await assertNamehash('evmreg')
})
})
76 changes: 27 additions & 49 deletions test/contracts/apm/apm_registry.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { hash } = require('eth-ens-namehash')
const { keccak_256 } = require('js-sha3')
const { assertRevert } = require('../../helpers/assertThrow')
const namehash = require('eth-ens-namehash').hash
const keccak256 = require('js-sha3').keccak_256
const { getEventArgument } = require('../../helpers/events')

const ENS = artifacts.require('ENS')
const ENSFactory = artifacts.require('ENSFactory')
Expand All @@ -14,23 +15,16 @@ const APMRegistry = artifacts.require('APMRegistry')
const ENSSubdomainRegistrar = artifacts.require('ENSSubdomainRegistrar')
const Repo = artifacts.require('Repo')
const APMRegistryFactory = artifacts.require('APMRegistryFactory')

const APMRegistryFactoryMock = artifacts.require('APMRegistryFactoryMock')

const getRepoFromLog = receipt => receipt.logs.filter(x => x.event == 'NewRepo')[0].args.repo

const EMPTY_BYTES = '0x'
const ZERO_ADDR = '0x0000000000000000000000000000000000000000'

contract('APMRegistry', accounts => {
contract('APMRegistry', ([ensOwner, apmOwner, repoDev, notOwner, someone]) => {
let baseDeployed, baseAddrs, ensFactory, apmFactory, daoFactory, ens, registry, acl
const ensOwner = accounts[0]
const apmOwner = accounts[1]
const repoDev = accounts[2]
const notOwner = accounts[5]

const rootNode = namehash('aragonpm.eth')
const testNode = namehash('test.aragonpm.eth')
const rootNode = hash('aragonpm.eth')
const testNode = hash('test.aragonpm.eth')

before(async () => {
const bases = [APMRegistry, Repo, ENSSubdomainRegistrar]
Expand All @@ -48,8 +42,8 @@ contract('APMRegistry', accounts => {
apmFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ZERO_ADDR, ensFactory.address)
ens = ENS.at(await apmFactory.ens())

const receipt = await apmFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner)
const apmAddr = receipt.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
const receipt = await apmFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner)
const apmAddr = getEventArgument(receipt, 'DeployAPM', 'apm')
registry = APMRegistry.at(apmAddr)

const dao = Kernel.at(await registry.kernel())
Expand All @@ -61,13 +55,13 @@ contract('APMRegistry', accounts => {
})

it('inits with existing ENS deployment', async () => {
const receipt = await ensFactory.newENS(accounts[0])
const ens2 = ENS.at(receipt.logs.filter(l => l.event == 'DeployENS')[0].args.ens)
const receipt = await ensFactory.newENS(ensOwner)
const ens2 = ENS.at(getEventArgument(receipt, 'DeployENS', 'ens'))
const newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, ZERO_ADDR)

await ens2.setSubnodeOwner(namehash('eth'), '0x'+keccak256('aragonpm'), newFactory.address)
const receipt2 = await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner)
const apmAddr = receipt2.logs.filter(l => l.event == 'DeployAPM')[0].args.apm
await ens2.setSubnodeOwner(hash('eth'), '0x'+keccak_256('aragonpm'), newFactory.address)
const receipt2 = await newFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner)
const apmAddr = getEventArgument(receipt2, 'DeployAPM', 'apm')
const resolver = PublicResolver.at(await ens2.resolver(rootNode))

assert.equal(await resolver.addr(rootNode), apmAddr, 'rootnode should be resolve')
Expand All @@ -85,7 +79,7 @@ contract('APMRegistry', accounts => {

it('can create repo with version and dev can create new versions', async () => {
const receipt = await registry.newRepoWithVersion('test', repoDev, [1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: apmOwner })
const repo = Repo.at(getRepoFromLog(receipt))
const repo = Repo.at(getEventArgument(receipt, 'NewRepo', 'repo'))

assert.equal(await repo.getVersionsCount(), 1, 'should have created version')

Expand All @@ -95,38 +89,32 @@ contract('APMRegistry', accounts => {
})

it('fails to init with existing ENS deployment if not owner of tld', async () => {
const ensReceipt = await ensFactory.newENS(accounts[0])
const ens2 = ENS.at(ensReceipt.logs.filter(l => l.event == 'DeployENS')[0].args.ens)
const ensReceipt = await ensFactory.newENS(ensOwner)
const ens2 = ENS.at(getEventArgument(ensReceipt, 'DeployENS', 'ens'))
const newFactory = await APMRegistryFactory.new(daoFactory.address, ...baseAddrs, ens2.address, ZERO_ADDR)

// Factory doesn't have ownership over 'eth' tld
await assertRevert(async () => {
await newFactory.newAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner)
})
await assertRevert(newFactory.newAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner))
})

it('fails to create empty repo name', async () => {
return assertRevert(async () => {
await registry.newRepo('', repoDev, { from: apmOwner })
})
await assertRevert(registry.newRepo('', repoDev, { from: apmOwner }))
})

it('fails to create repo if not in ACL', async () => {
return assertRevert(async () => {
await registry.newRepo('test', repoDev, { from: notOwner })
})
await assertRevert(registry.newRepo('test', repoDev, { from: notOwner }))
})

context('> Creating test.aragonpm.eth repo', () => {
let repo = {}

beforeEach(async () => {
const receipt = await registry.newRepo('test', repoDev, { from: apmOwner })
repo = Repo.at(getRepoFromLog(receipt))
repo = Repo.at(getEventArgument(receipt, 'NewRepo', 'repo'))
})

it('resolver is setup correctly', async () => {
const resolverNode = namehash('resolver.eth')
const resolverNode = hash('resolver.eth')
const publicResolver = PublicResolver.at(await ens.resolver(resolverNode))

assert.equal(await ens.resolver(testNode), await publicResolver.addr(resolverNode), 'resolver should be set to public resolver')
Expand All @@ -138,9 +126,7 @@ contract('APMRegistry', accounts => {
})

it('fails when creating repo with existing name', async () => {
return assertRevert(async () => {
await registry.newRepo('test', repoDev)
})
await assertRevert(registry.newRepo('test', repoDev))
})

it('repo dev can create versions', async () => {
Expand All @@ -152,7 +138,7 @@ contract('APMRegistry', accounts => {

it('repo dev can authorize someone to interact with repo', async () => {
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })
const newOwner = accounts[8]
const newOwner = someone

await acl.grantPermission(newOwner, repo.address, await repo.CREATE_VERSION_ROLE(), { from: repoDev })

Expand All @@ -166,15 +152,11 @@ contract('APMRegistry', accounts => {
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })
await acl.revokePermission(repoDev, repo.address, await repo.CREATE_VERSION_ROLE(), { from: repoDev })

return assertRevert(async () => {
await repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev })
})
await assertRevert(repo.newVersion([2, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: repoDev }))
})

it('cannot create versions if not in ACL', async () => {
return assertRevert(async () => {
await repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: notOwner })
})
await assertRevert(repo.newVersion([1, 0, 0], ZERO_ADDR, EMPTY_BYTES, { from: notOwner }))
})
})

Expand All @@ -186,15 +168,11 @@ contract('APMRegistry', accounts => {
})

it('fails if factory doesnt give permission to create names', async () => {
await assertRevert(async () => {
await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, true)
})
await assertRevert(apmFactoryMock.newFailingAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner, true))
})

it('fails if factory doesnt give permission to create permissions', async () => {
await assertRevert(async () => {
await apmFactoryMock.newFailingAPM(namehash('eth'), '0x'+keccak256('aragonpm'), apmOwner, false)
})
await assertRevert(apmFactoryMock.newFailingAPM(hash('eth'), '0x'+keccak_256('aragonpm'), apmOwner, false))
})
})
})
18 changes: 5 additions & 13 deletions test/contracts/apm/apm_repo.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ contract('Repo', accounts => {

beforeEach(async () => {
repo = await Repo.new()
await repo.initialize();
await repo.initialize()
})

it('computes correct valid bumps', async () => {
Expand All @@ -32,9 +32,7 @@ contract('Repo', accounts => {

// valid version as being a correct bump from 0.0.0
it('cannot create invalid first version', async () => {
return assertRevert(async () => {
await repo.newVersion([1, 1, 0], ZERO_ADDR, EMPTY_BYTES)
})
await assertRevert(repo.newVersion([1, 1, 0], ZERO_ADDR, EMPTY_BYTES))
})

context('creating initial version', () => {
Expand Down Expand Up @@ -78,21 +76,15 @@ contract('Repo', accounts => {
})

it('fails when changing contract address in non major version', async () => {
return assertRevert(async () => {
await repo.newVersion([1, 1, 0], accounts[2], initialContent)
})
await assertRevert(repo.newVersion([1, 1, 0], accounts[2], initialContent))
})

it('fails when version bump is invalid', async () => {
return assertRevert(async () => {
await repo.newVersion([1, 2, 0], initialCode, initialContent)
})
await assertRevert(repo.newVersion([1, 2, 0], initialCode, initialContent))
})

it('fails if requesting version 0', async () => {
return assertRevert(async () => {
await repo.getByVersionId(0)
})
await assertRevert(repo.getByVersionId(0))
})

context('adding new version', () => {
Expand Down
16 changes: 5 additions & 11 deletions test/contracts/apps/app_acl.js
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
const { assertRevert } = require('../../helpers/assertThrow')
const { onlyIf } = require('../../helpers/onlyIf')
const { hash } = require('eth-ens-namehash')
const { onlyIf } = require('../../helpers/onlyIf')
const { assertRevert } = require('../../helpers/assertThrow')

const ACL = artifacts.require('ACL')
const Kernel = artifacts.require('Kernel')
Expand Down Expand Up @@ -95,9 +95,7 @@ contract('App ACL', accounts => {
})

it('fails when called by unauthorized entity', async () => {
return assertRevert(async () => {
await app.setValue(10, { from: unauthorized })
})
await assertRevert(app.setValue(10, { from: unauthorized }))
})

onlyAppProxyUpgradeable(() =>
Expand All @@ -106,9 +104,7 @@ contract('App ACL', accounts => {
const appProxy = await AppProxyUpgradeable.new(kernel.address, unknownId, EMPTY_BYTES)
const app = AppStub.at(appProxy.address)

return assertRevert(async () => {
await app.setValue(10)
})
await assertRevert(app.setValue(10))
})
)

Expand Down Expand Up @@ -138,9 +134,7 @@ contract('App ACL', accounts => {
})

it('parametrized app call fails if param eval fails', async () => {
return assertRevert(async () => {
await app.setValueParam(failValue, { from: paramsGrantee })
})
await assertRevert(app.setValueParam(failValue, { from: paramsGrantee }))
})
})
})
Expand Down
14 changes: 4 additions & 10 deletions test/contracts/apps/app_base_lifecycle.js
View file
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
const { assertRevert } = require('../../helpers/assertThrow')
const { getBlockNumber } = require('../../helpers/web3')
const { hash } = require('eth-ens-namehash')
const { soliditySha3 } = require('web3-utils')
const { getBlockNumber } = require('../../helpers/web3')
const { assertRevert } = require('../../helpers/assertThrow')

const ACL = artifacts.require('ACL')
const Kernel = artifacts.require('Kernel')
const KernelProxy = artifacts.require('KernelProxy')

const AragonApp = artifacts.require('AragonApp')
const AppProxyUpgradeable = artifacts.require('AppProxyUpgradeable')

// Mocks
const UnsafeAragonAppMock = artifacts.require('UnsafeAragonAppMock')

const APP_ID = hash('app.aragonpm.test')
const FAKE_ROLE = soliditySha3('FAKE_ROLE')
const ZERO_ADDR = '0x0000000000000000000000000000000000000000'

contract('App base lifecycle', accounts => {
contract('App base lifecycle', ([permissionsRoot]) => {
let aclBase, kernelBase
const permissionsRoot = accounts[0]

before(async () => {
kernelBase = await Kernel.new(true) // petrify immediately
Expand Down Expand Up @@ -88,9 +84,7 @@ contract('App base lifecycle', accounts => {
})

it('throws on reinitialization', async () => {
return assertRevert(async () => {
await app.initialize()
})
await assertRevert(app.initialize())
})

it('should still not be usable without a kernel', async () => {
Expand Down
14 changes: 5 additions & 9 deletions test/contracts/apps/app_funds.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const { hash } = require('eth-ens-namehash')
const { assertRevert } = require('../../helpers/assertThrow')
const { getBalance } = require('../../helpers/web3')
const { onlyIf } = require('../../helpers/onlyIf')
const { getBalance } = require('../../helpers/web3')
const { assertRevert } = require('../../helpers/assertThrow')

const ACL = artifacts.require('ACL')
const Kernel = artifacts.require('Kernel')
Expand All @@ -19,12 +19,10 @@ const APP_ID = hash('stub.aragonpm.test')
const EMPTY_BYTES = '0x'
const SEND_ETH_GAS = 31000 // 21k base tx cost + 10k limit on depositable proxies

contract('App funds', accounts => {
contract('App funds', ([permissionsRoot]) => {
let aclBase, kernelBase
let APP_BASES_NAMESPACE

const permissionsRoot = accounts[0]

before(async () => {
kernelBase = await Kernel.new(true) // petrify immediately
aclBase = await ACL.new()
Expand Down Expand Up @@ -84,15 +82,13 @@ contract('App funds', accounts => {
app = appBaseType.at(appProxy.address)
}

await app.initialize();
await app.initialize()
})

it('cannot receive ETH', async () => {
assert.isTrue(await app.hasInitialized(), 'should have been initialized')

await assertRevert(async () => {
await app.sendTransaction({ value: 1, gas: SEND_ETH_GAS })
})
await assertRevert(app.sendTransaction({ value: 1, gas: SEND_ETH_GAS }))
})

onlyAppStubDepositable(() => {
Expand Down
Loading

0 comments on commit d10d195

Please sign in to comment.