Fix PR comments

aragon · Aug 22, 2018 · f4048ef · f4048ef
1 parent ff3c9d9
commit f4048ef
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 21 deletions.
diff --git a/test/apm_repo.js b/test/apm_repo.js
@@ -93,7 +93,7 @@ contract('Repo', accounts => {
             })
         })
 
-        context('adding new version', async () => {
+        context('adding new version', () => {
             const newCode = accounts[9] // random addr, irrelevant
             const newContent = '0x13'
 

diff --git a/test/evm_script.js b/test/evm_script.js
@@ -14,7 +14,7 @@ const IEVMScriptExecutor = artifacts.require('IEVMScriptExecutor')
 
 // Mocks
 const ExecutionTarget = artifacts.require('ExecutionTarget')
-const EVMScriptExecutor = artifacts.require('EVMScriptExecutor')
+const MockScriptExecutorApp = artifacts.require('MockScriptExecutorApp')
 
 const ZERO_ADDR = '0x0000000000000000000000000000000000000000'
 
@@ -44,7 +44,7 @@ contract('EVM Script', accounts => {
         reg = EVMScriptRegistry.at(receipt.logs.filter(l => l.event == 'DeployEVMScriptRegistry')[0].args.reg)
 
         await acl.createPermission(boss, dao.address, await dao.APP_MANAGER_ROLE(), boss, { from: boss })
-        executorBase = await EVMScriptExecutor.new()
+        executorBase = await MockScriptExecutorApp.new()
         await dao.setApp(APP_BASES_NAMESPACE, executorAppId, executorBase.address, { from: boss })
     })
 
@@ -65,13 +65,13 @@ contract('EVM Script', accounts => {
         const action = { to: executionTarget.address, calldata: executionTarget.contract.execute.getData() }
         const script = encodeCallScript([action])
 
-        assertRevert(() => callsScriptBase.execScript(script, '0x', []))
+        await assertRevert(() => callsScriptBase.execScript(script, '0x', []))
     })
 
     context('> Uninitialized executor', () => {
         beforeEach(async () => {
             const receipt = await dao.newAppInstance(executorAppId, executorBase.address, { from: boss })
-            executor = EVMScriptExecutor.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
+            executor = MockScriptExecutorApp.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
             // Explicitly don't initialize the executor
             executionTarget = await ExecutionTarget.new()
         })
@@ -87,7 +87,7 @@ contract('EVM Script', accounts => {
     context('> Executor', () => {
         beforeEach(async () => {
             const receipt = await dao.newAppInstance(executorAppId, executorBase.address, { from: boss })
-            executor = EVMScriptExecutor.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
+            executor = MockScriptExecutorApp.at(receipt.logs.filter(l => l.event == 'NewAppProxy')[0].args.proxy)
             await executor.initialize()
             executionTarget = await ExecutionTarget.new()
         })
@@ -203,7 +203,7 @@ contract('EVM Script', accounts => {
                 await executor.execute(encodeCallScript([]))
             })
 
-            context('> Script overflow', async () => {
+            context('> Script overflow', () => {
                 const encodeCallScriptBad = actions => {
                     return actions.reduce((script, { to, calldata }) => {
                         const addr = rawEncode(['address'], [to]).toString('hex')

diff --git a/test/kernel_lifecycle.js b/test/kernel_lifecycle.js
@@ -17,7 +17,7 @@ contract('Kernel lifecycle', accounts => {
   let ACL_APP_ID, APP_BASES_NAMESPACE, APP_ADDR_NAMESPACE, APP_MANAGER_ROLE
 
   const testUnaccessibleFunctionalityWhenUninitialized = async (kernel) => {
-    // hasPermission should always return false when unintialized
+    // hasPermission should always return false when uninitialized
     assert.isFalse(await kernel.hasPermission(accounts[0], kernel.address, APP_MANAGER_ROLE, '0x'))
     assert.isFalse(await kernel.hasPermission(accounts[1], kernel.address, APP_MANAGER_ROLE, '0x'))
 

diff --git a/test/mocks/EVMScriptExecutor.sol → test/mocks/MockScriptExecutorApp.sol b/test/mocks/EVMScriptExecutor.sol → test/mocks/MockScriptExecutorApp.sol
@@ -3,7 +3,7 @@ pragma solidity 0.4.18;
 import "../../contracts/apps/AragonApp.sol";
 
 
-contract EVMScriptExecutor is AragonApp {
+contract MockScriptExecutorApp is AragonApp {
     // Initialization is required to access any of the real executors
     function initialize() public {
         initialized();

diff --git a/test/proxy_recover_funds.js b/test/proxy_recover_funds.js
@@ -106,8 +106,11 @@ contract('Proxy funds', accounts => {
       // Test both the Vault itself and when it's behind a proxy to make sure their recovery behaviours are the same
       for (const vaultType of ['Vault', 'VaultProxy']) {
         const skipCoverageIfVaultProxy = test => {
-          // The VaultMock isn't instrumented during coverage, but the AppProxy is, and so transferring
-          // to the fallback fails when we're testing with the proxy
+          // The VaultMock isn't instrumented during coverage, but the AppProxy is, and so
+          // transferring to the fallback fails when we're testing with the proxy.
+          // Native transfers (either .send() or .transfer()) fail under coverage because they're
+          // limited to 2.3k gas, and the injected instrumentation from coverage makes these
+          // operations cost more than that limit.
           return vaultType === 'VaultProxy' ? skipCoverage(test) : test
         }
 
@@ -131,9 +134,9 @@ contract('Proxy funds', accounts => {
           })
 
           onlyBaseKernel(() => {
-            context('> Base kernel', async () => {
+            context('> Base kernel', () => {
               it('cannot receive ETH', skipCoverageIfVaultProxy(async () =>
-                assertRevert(
+                await assertRevert(
                   () => kernel.sendTransaction({ value: 1, gas: 31000 })
                 )
               ))
@@ -145,7 +148,7 @@ contract('Proxy funds', accounts => {
           })
 
           onlyKernelProxy(() => {
-            context('> KernelProxy', async () => {
+            context('> KernelProxy', () => {
               beforeEach(() => {
                 target = kernel
               })
@@ -164,25 +167,25 @@ contract('Proxy funds', accounts => {
             })
           })
 
-          context('> App without kernel', async () => {
+          context('> App without kernel', () => {
             beforeEach(async () => {
               target = await AppStub.new()
             })
 
-            it('does not recover ETH', skipCoverageIfVaultProxy(() =>
-              assertRevert(
+            it('does not recover ETH', skipCoverageIfVaultProxy(async () =>
+              await assertRevert(
                 () => recoverEth(target, vault)
               )
             ))
 
-            it('does not recover tokens', () =>
-              assertRevert(
+            it('does not recover tokens', async () =>
+              await assertRevert(
                 () => recoverTokens(target, vault)
               )
             )
           })
 
-          context('> Proxied app with kernel', async () => {
+          context('> Proxied app with kernel', () => {
             beforeEach(async () => {
               // Setup app
               const receipt = await kernel.newAppInstance(APP_ID, appBase.address)
@@ -215,7 +218,7 @@ contract('Proxy funds', accounts => {
             })
           })
 
-          context('> Conditional fund recovery', async () => {
+          context('> Conditional fund recovery', () => {
             beforeEach(async () => {
               // Setup app with conditional recovery code
               const receipt = await kernel.newAppInstance(APP_ID, appConditionalRecoveryBase.address)