Now testing events in constructors! (#1511)

* Added inTransaction tests.

* Added expectEvent.inConstructor.

* Changed inTransaction, removed decodeLogs.

* Flipped comparison to improve the error message.

* Improved expectEvent tests.

* Migrated tests to use expectEvent.

* Added roles constructor tests.

* Fixed linter errors.

* Made lodash a dev dependency.

* Added more inLogs tests.

* Update expectEvent.test.js

* Removed lodash.

* Moved role constructor tests to public role behavior.

* Revert "Flipped comparison to improve the error message."

This reverts commit 438c57833d997d2972a7730180a266bc3c6a87c5.

* Replaced chai-as-promised with shouldFail.

contracts/mocks/EventEmitter.sol

@@ -11,6 +11,12 @@ contract EventEmitter {
     event String(string value);
     event LongUintBooleanString(uint256 uintValue, bool booleanValue, string stringValue);
 
+    constructor (uint8 uintValue, bool booleanValue, string stringValue) public {
+        emit ShortUint(uintValue);
+        emit Boolean(booleanValue);
+        emit String(stringValue);
+    }
+
     function emitArgumentless() public {
         emit Argumentless();
     }
@@ -51,4 +57,17 @@ contract EventEmitter {
         emit LongUint(uintValue);
         emit Boolean(boolValue);
     }
+
+    function emitStringAndEmitIndirectly(string value, IndirectEventEmitter emitter) public {
+        emit String(value);
+        emitter.emitStringIndirectly(value);
+    }
+}
+
+contract IndirectEventEmitter {
+    event IndirectString(string value);
+
+    function emitStringIndirectly(string value) public {
+        emit IndirectString(value);
+    }
 }

package-lock.json

@@ -6885,9 +6885,9 @@
       }
     },
     "lodash": {
-      "version": "4.17.10",
-      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.10.tgz",
-      "integrity": "sha512-UejweD1pDoXu+AD825lWwp4ZGtSwgnpZxb3JDViD7StjQz+Nb/6l093lx4OQ0foGWNRoc19mWy7BzL+UAK2iVg==",
+      "version": "4.17.11",
+      "resolved": "https://registry.npmjs.org/lodash/-/lodash-4.17.11.tgz",
+      "integrity": "sha512-cQKh8igo5QUhZ7lg38DYWAxMvjSAKG0A8wGSVimP07SIUEK2UO+arSRKbRZWtelMtN5V0Hkwh5ryOto/SshYIg==",
       "dev": true
     },
     "lodash.assign": {

package.json

@@ -55,5 +55,6 @@
     "truffle": "^4.1.13",
     "truffle-hdwallet-provider": "0.0.5",
     "web3-utils": "^1.0.0-beta.34"
-  }
+  },
+  "dependencies": {}
 }

test/access/roles/PublicRole.behavior.js

@@ -19,6 +19,12 @@ function shouldBehaveLikePublicRole (authorized, otherAuthorized, [anyone], role
       (await this.contract[`is${rolename}`](anyone)).should.equal(false);
     });
 
+    it('emits events during construction', async function () {
+      await expectEvent.inConstruction(this.contract, `${rolename}Added`, {
+        account: authorized,
+      });
+    });
+
     it('reverts when querying roles for the null account', async function () {
       await shouldFail.reverting(this.contract[`is${rolename}`](ZERO_ADDRESS));
     });

test/examples/SimpleToken.test.js

@@ -1,4 +1,4 @@
-const { decodeLogs } = require('../helpers/decodeLogs');
+const expectEvent = require('../helpers/expectEvent');
 const { ZERO_ADDRESS } = require('../helpers/constants');
 const SimpleToken = artifacts.require('SimpleToken');
 
@@ -31,12 +31,10 @@ contract('SimpleToken', function ([_, creator]) {
 
     creatorBalance.should.be.bignumber.equal(totalSupply);
 
-    const receipt = await web3.eth.getTransactionReceipt(this.token.transactionHash);
-    const logs = decodeLogs(receipt.logs, SimpleToken, this.token.address);
-    logs.length.should.equal(1);
-    logs[0].event.should.equal('Transfer');
-    logs[0].args.from.valueOf().should.equal(ZERO_ADDRESS);
-    logs[0].args.to.valueOf().should.equal(creator);
-    logs[0].args.value.should.be.bignumber.equal(totalSupply);
+    await expectEvent.inConstruction(this.token, 'Transfer', {
+      from: ZERO_ADDRESS,
+      to: creator,
+      value: totalSupply,
+    });
   });
 });

test/helpers/decodeLogs.js

@@ -1,12 +0,0 @@
-const SolidityEvent = require('web3/lib/web3/event.js');
-
-function decodeLogs (logs, contract, address) {
-  return logs.map(log => {
-    const event = new SolidityEvent(null, contract.events[log.topics[0]], address);
-    return event.decode(log);
-  });
-}
-
-module.exports = {
-  decodeLogs,
-};

test/helpers/expectEvent.js

@@ -1,3 +1,5 @@
+const SolidityEvent = require('web3/lib/web3/event.js');
+
 const BigNumber = web3.BigNumber;
 const should = require('chai')
   .use(require('chai-bignumber')(BigNumber))
@@ -16,8 +18,14 @@ function inLogs (logs, eventName, eventArgs = {}) {
   return event;
 }
 
-async function inTransaction (tx, eventName, eventArgs = {}) {
-  const { logs } = await tx;
+async function inConstruction (contract, eventName, eventArgs = {}) {
+  return inTransaction(contract.transactionHash, contract.constructor, eventName, eventArgs);
+}
+
+async function inTransaction (txHash, emitter, eventName, eventArgs = {}) {
+  const receipt = await web3.eth.getTransactionReceipt(txHash);
+  const logs = decodeLogs(receipt.logs, emitter.events);
+
   return inLogs(logs, eventName, eventArgs);
 }
 
@@ -35,7 +43,17 @@ function isBigNumber (object) {
     (object.constructor && object.constructor.name === 'BigNumber');
 }
 
+function decodeLogs (logs, events) {
+  return Array.prototype.concat(...logs.map(log =>
+    log.topics.filter(topic => topic in events).map(topic => {
+      const event = new SolidityEvent(null, events[topic], 0);
+      return event.decode(log);
+    })
+  ));
+}
+
 module.exports = {
   inLogs,
+  inConstruction,
   inTransaction,
 };

test/helpers/test/expectEvent.test.js

@@ -1,5 +1,8 @@
 const expectEvent = require('../expectEvent');
+const shouldFail = require('../shouldFail');
+
 const EventEmitter = artifacts.require('EventEmitter');
+const IndirectEventEmitter = artifacts.require('IndirectEventEmitter');
 
 const BigNumber = web3.BigNumber;
 const should = require('chai')
@@ -8,7 +11,57 @@ const should = require('chai')
 
 describe('expectEvent', function () {
   beforeEach(async function () {
-    this.emitter = await EventEmitter.new();
+    this.constructionValues = {
+      uint: 42,
+      boolean: true,
+      string: 'OpenZeppelin',
+    };
+
+    this.emitter = await EventEmitter.new(
+      this.constructionValues.uint,
+      this.constructionValues.boolean,
+      this.constructionValues.string
+    );
+  });
+
+  describe('inConstructor', function () {
+    context('short uint value', function () {
+      it('accepts emitted events with correct number', async function () {
+        await expectEvent.inConstruction(this.emitter, 'ShortUint',
+          { value: this.constructionValues.uint }
+        );
+      });
+
+      it('throws if an incorrect value is passed', async function () {
+        await shouldFail(expectEvent.inConstruction(this.emitter, 'ShortUint', { value: 23 }));
+      });
+    });
+
+    context('boolean value', function () {
+      it('accepts emitted events with correct value', async function () {
+        await expectEvent.inConstruction(this.emitter, 'Boolean', { value: this.constructionValues.boolean });
+      });
+
+      it('throws if an incorrect value is passed', async function () {
+        await shouldFail(expectEvent.inConstruction(this.emitter, 'Boolean',
+          { value: !this.constructionValues.boolean }
+        ));
+      });
+    });
+
+    context('string value', function () {
+      it('accepts emitted events with correct string', async function () {
+        await expectEvent.inConstruction(this.emitter, 'String', { value: this.constructionValues.string });
+      });
+
+      it('throws if an incorrect string is passed', async function () {
+        await shouldFail(expectEvent.inConstruction(this.emitter, 'String', { value: 'ClosedZeppelin' }));
+      });
+    });
+
+    it('throws if an unemitted event is requested', async function () {
+      await shouldFail(expectEvent.inConstruction(this.emitter, 'UnemittedEvent'));
+    });
   });
 
   describe('inLogs', function () {
@@ -228,5 +281,98 @@ describe('expectEvent', function () {
         should.Throw(() => expectEvent.inLogs(this.logs, 'Boolean', { value: false }));
       });
     });
+
+    describe('with events emitted by an indirectly called contract', function () {
+      beforeEach(async function () {
+        this.secondEmitter = await IndirectEventEmitter.new();
+
+        this.value = 'OpenZeppelin';
+        ({ logs: this.logs } = await this.emitter.emitStringAndEmitIndirectly(this.value, this.secondEmitter.address));
+      });
+
+      it('accepts events emitted by the directly called contract', function () {
+        expectEvent.inLogs(this.logs, 'String', { value: this.value });
+      });
+
+      it('throws when passing events emitted by the indirectly called contract', function () {
+        should.Throw(() => expectEvent.inLogs(this.logs, 'IndirectString', { value: this.value }));
+      });
+    });
+  });
+
+  describe('inTransaction', function () {
+    describe('when emitting from called contract and indirect calls', function () {
+      context('string value', function () {
+        beforeEach(async function () {
+          this.secondEmitter = await IndirectEventEmitter.new();
+
+          this.value = 'OpenZeppelin';
+          const receipt = await this.emitter.emitStringAndEmitIndirectly(this.value, this.secondEmitter.address);
+          this.txHash = receipt.tx;
+        });
+
+        context('with directly called contract', function () {
+          it('accepts emitted events with correct string', async function () {
+            await expectEvent.inTransaction(this.txHash, EventEmitter, 'String', { value: this.value });
+          });
+
+          it('throws if an unemitted event is requested', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, EventEmitter, 'UnemittedEvent',
+              { value: this.value }
+            ));
+          });
+
+          it('throws if an incorrect string is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, EventEmitter, 'String',
+              { value: 'ClosedZeppelin' }
+            ));
+          });
+
+          it('throws if an event emitted from other contract is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, EventEmitter, 'IndirectString',
+              { value: this.value }
+            ));
+          });
+
+          it('throws if an incorrect emitter is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, IndirectEventEmitter, 'String',
+              { value: this.value }
+            ));
+          });
+        });
+
+        context('with indirectly called contract', function () {
+          it('accepts events emitted from other contracts', async function () {
+            await expectEvent.inTransaction(this.txHash, IndirectEventEmitter, 'IndirectString',
+              { value: this.value }
+            );
+          });
+
+          it('throws if an unemitted event is requested', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, IndirectEventEmitter, 'UnemittedEvent',
+              { value: this.value }
+            ));
+          });
+
+          it('throws if an incorrect string is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, IndirectEventEmitter, 'IndirectString',
+              { value: 'ClosedZeppelin' }
+            ));
+          });
+
+          it('throws if an event emitted from other contract is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, IndirectEventEmitter, 'String',
+              { value: this.value }
+            ));
+          });
+
+          it('throws if an incorrect emitter is passed', async function () {
+            await shouldFail(expectEvent.inTransaction(this.txHash, EventEmitter, 'IndirectString',
+              { value: this.value }
+            ));
+          });
+        });
+      });
+    });
   });
 });

test/token/ERC721/ERC721.behavior.js

@@ -2,7 +2,6 @@ const expectEvent = require('../../helpers/expectEvent');
 const { shouldSupportInterfaces } = require('../../introspection/SupportsInterface.behavior');
 const shouldFail = require('../../helpers/shouldFail');
 const { ZERO_ADDRESS } = require('../../helpers/constants');
-const { decodeLogs } = require('../../helpers/decodeLogs');
 const { sendTransaction } = require('../../helpers/sendTransaction');
 
 const ERC721ReceiverMock = artifacts.require('ERC721ReceiverMock.sol');
@@ -245,31 +244,25 @@ function shouldBehaveLikeERC721 (
             shouldTransferTokensByUsers(transferFun);
 
             it('should call onERC721Received', async function () {
-              const result = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
-              result.receipt.logs.length.should.be.equal(2);
-              const [log] = decodeLogs([result.receipt.logs[1]], ERC721ReceiverMock, this.receiver.address);
-              log.event.should.be.equal('Received');
-              log.args.operator.should.be.equal(owner);
-              log.args.from.should.be.equal(owner);
-              log.args.tokenId.toNumber().should.be.equal(tokenId);
-              log.args.data.should.be.equal(data);
+              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: owner });
+
+              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+                operator: owner,
+                from: owner,
+                tokenId: tokenId,
+                data: data,
+              });
             });
 
             it('should call onERC721Received from approved', async function () {
-              const result = await transferFun.call(this, owner, this.receiver.address, tokenId, {
-                from: approved,
+              const receipt = await transferFun.call(this, owner, this.receiver.address, tokenId, { from: approved });
+
+              await expectEvent.inTransaction(receipt.tx, ERC721ReceiverMock, 'Received', {
+                operator: approved,
+                from: owner,
+                tokenId: tokenId,
+                data: data,
               });
-              result.receipt.logs.length.should.be.equal(2);
-              const [log] = decodeLogs(
-                [result.receipt.logs[1]],
-                ERC721ReceiverMock,
-                this.receiver.address
-              );
-              log.event.should.be.equal('Received');
-              log.args.operator.should.be.equal(approved);
-              log.args.from.should.be.equal(owner);
-              log.args.tokenId.toNumber().should.be.equal(tokenId);
-              log.args.data.should.be.equal(data);
             });
 
             describe('with an invalid token id', function () {