Remove `async` from `describe` blocks and add missing `await` in tests (#4942)

Co-authored-by: ernestognw <ernestognw@gmail.com>
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>

hardhat/async-test-sanity.js

@@ -0,0 +1,3 @@
+process.on('unhandledRejection', reason => {
+  throw new Error(reason);
+});

test/access/AccessControl.behavior.js

@@ -43,7 +43,7 @@ function shouldBehaveLikeAccessControl() {
 
     it('accounts can be granted a role multiple times', async function () {
       await this.mock.connect(this.defaultAdmin).grantRole(ROLE, this.authorized);
-      expect(this.mock.connect(this.defaultAdmin).grantRole(ROLE, this.authorized)).to.not.emit(
+      await expect(this.mock.connect(this.defaultAdmin).grantRole(ROLE, this.authorized)).to.not.emit(
         this.mock,
         'RoleGranted',
       );
@@ -82,7 +82,7 @@ function shouldBehaveLikeAccessControl() {
       it('a role can be revoked multiple times', async function () {
         await this.mock.connect(this.defaultAdmin).revokeRole(ROLE, this.authorized);
 
-        expect(this.mock.connect(this.defaultAdmin).revokeRole(ROLE, this.authorized)).to.not.emit(
+        await expect(this.mock.connect(this.defaultAdmin).revokeRole(ROLE, this.authorized)).to.not.emit(
           this.mock,
           'RoleRevoked',
         );
@@ -112,10 +112,9 @@ function shouldBehaveLikeAccessControl() {
       });
 
       it('only the sender can renounce their roles', async function () {
-        expect(this.mock.connect(this.defaultAdmin).renounceRole(ROLE, this.authorized)).to.be.revertedWithCustomError(
-          this.mock,
-          'AccessControlBadConfirmation',
-        );
+        await expect(
+          this.mock.connect(this.defaultAdmin).renounceRole(ROLE, this.authorized),
+        ).to.be.revertedWithCustomError(this.mock, 'AccessControlBadConfirmation');
       });
 
       it('a role can be renounced multiple times', async function () {
@@ -571,7 +570,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
       ]) {
         it(`should revert if block.timestamp is ${tag} to schedule`, async function () {
           await time.increaseTo.timestamp(this.acceptSchedule + fromSchedule, false);
-          expect(this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer())
+          await expect(this.mock.connect(this.newDefaultAdmin).acceptDefaultAdminTransfer())
             .to.be.revertedWithCustomError(this.mock, 'AccessControlEnforcedDefaultAdminDelay')
             .withArgs(this.acceptSchedule);
         });
@@ -625,7 +624,7 @@ function shouldBehaveLikeAccessControlDefaultAdminRules() {
       });
     });
 
-    describe('when there is no pending default admin transfer', async function () {
+    describe('when there is no pending default admin transfer', function () {
       it('should succeed without changes', async function () {
         await expect(this.mock.connect(this.defaultAdmin).cancelDefaultAdminTransfer()).to.not.emit(
           this.mock,

test/access/Ownable2Step.test.js

@@ -48,7 +48,7 @@ describe('Ownable2Step', function () {
     });
   });
 
-  describe('renouncing ownership', async function () {
+  describe('renouncing ownership', function () {
     it('changes owner after renouncing ownership', async function () {
       await expect(this.ownable2Step.connect(this.owner).renounceOwnership())
         .to.emit(this.ownable2Step, 'OwnershipTransferred')

test/access/manager/AccessManager.test.js

@@ -887,8 +887,8 @@ describe('AccessManager', function () {
           shouldBehaveLikeDelayedAdminOperation();
         });
 
-        it('reverts setting grant delay for the PUBLIC_ROLE', function () {
-          expect(this.manager.connect(this.admin).setGrantDelay(this.roles.PUBLIC.id, 69n))
+        it('reverts setting grant delay for the PUBLIC_ROLE', async function () {
+          await expect(this.manager.connect(this.admin).setGrantDelay(this.roles.PUBLIC.id, 69n))
             .to.be.revertedWithCustomError(this.manager, 'AccessManagerLockedRole')
             .withArgs(this.roles.PUBLIC.id);
         });
@@ -907,7 +907,7 @@ describe('AccessManager', function () {
           it('increases the delay after minsetback', async function () {
             const txResponse = await this.manager.connect(this.admin).setGrantDelay(this.role.id, newDelay);
             const setGrantDelayAt = await time.clockFromReceipt.timestamp(txResponse);
-            expect(txResponse)
+            await expect(txResponse)
               .to.emit(this.manager, 'RoleGrantDelayChanged')
               .withArgs(this.role.id, newDelay, setGrantDelayAt + MINSETBACK);
 
@@ -933,7 +933,7 @@ describe('AccessManager', function () {
             it('increases the delay after minsetback', async function () {
               const txResponse = await this.manager.connect(this.admin).setGrantDelay(this.role.id, newDelay);
               const setGrantDelayAt = await time.clockFromReceipt.timestamp(txResponse);
-              expect(txResponse)
+              await expect(txResponse)
                 .to.emit(this.manager, 'RoleGrantDelayChanged')
                 .withArgs(this.role.id, newDelay, setGrantDelayAt + MINSETBACK);
 
@@ -956,7 +956,7 @@ describe('AccessManager', function () {
               const txResponse = await this.manager.connect(this.admin).setGrantDelay(this.role.id, newDelay);
               const setGrantDelayAt = await time.clockFromReceipt.timestamp(txResponse);
 
-              expect(txResponse)
+              await expect(txResponse)
                 .to.emit(this.manager, 'RoleGrantDelayChanged')
                 .withArgs(this.role.id, newDelay, setGrantDelayAt + setback);
 
@@ -992,7 +992,7 @@ describe('AccessManager', function () {
           it('increases the delay after minsetback', async function () {
             const txResponse = await this.manager.connect(this.admin).setTargetAdminDelay(this.other, newDelay);
             const setTargetAdminDelayAt = await time.clockFromReceipt.timestamp(txResponse);
-            expect(txResponse)
+            await expect(txResponse)
               .to.emit(this.manager, 'TargetAdminDelayUpdated')
               .withArgs(this.other, newDelay, setTargetAdminDelayAt + MINSETBACK);
 
@@ -1017,7 +1017,7 @@ describe('AccessManager', function () {
             it('increases the delay after minsetback', async function () {
               const txResponse = await this.manager.connect(this.admin).setTargetAdminDelay(this.other, newDelay);
               const setTargetAdminDelayAt = await time.clockFromReceipt.timestamp(txResponse);
-              expect(txResponse)
+              await expect(txResponse)
                 .to.emit(this.manager, 'TargetAdminDelayUpdated')
                 .withArgs(this.other, newDelay, setTargetAdminDelayAt + MINSETBACK);
 
@@ -1040,7 +1040,7 @@ describe('AccessManager', function () {
               const txResponse = await this.manager.connect(this.admin).setTargetAdminDelay(this.other, newDelay);
               const setTargetAdminDelayAt = await time.clockFromReceipt.timestamp(txResponse);
 
-              expect(txResponse)
+              await expect(txResponse)
                 .to.emit(this.manager, 'TargetAdminDelayUpdated')
                 .withArgs(this.other, newDelay, setTargetAdminDelayAt + setback);
 
@@ -1135,7 +1135,7 @@ describe('AccessManager', function () {
             .setTargetFunctionRole(this.target, sigs, this.roles.SOME.id);
 
           for (const sig of sigs) {
-            expect(allowRole)
+            await expect(allowRole)
               .to.emit(this.manager, 'TargetFunctionRoleUpdated')
               .withArgs(this.target, sig, this.roles.SOME.id);
             expect(await this.manager.getTargetFunctionRole(this.target, sig)).to.equal(this.roles.SOME.id);
@@ -1212,9 +1212,9 @@ describe('AccessManager', function () {
 
                   it('does not grant role to the user yet', async function () {
                     const timestamp = await time.clockFromReceipt.timestamp(this.txResponse);
-                    expect(this.txResponse)
+                    await expect(this.txResponse)
                       .to.emit(this.manager, 'RoleGranted')
-                      .withArgs(ANOTHER_ROLE, this.user, timestamp + this.grantDelay, this.executionDelay, true);
+                      .withArgs(ANOTHER_ROLE, this.user, this.executionDelay, timestamp + this.grantDelay, true);
 
                     // Access is correctly stored
                     const access = await this.manager.getAccess(ANOTHER_ROLE, this.user);
@@ -1237,9 +1237,9 @@ describe('AccessManager', function () {
 
                   it('grants role to the user', async function () {
                     const timestamp = await time.clockFromReceipt.timestamp(this.txResponse);
-                    expect(this.txResponse)
-                      .to.emit(this.manager, 'RoleAccessRequested')
-                      .withArgs(ANOTHER_ROLE, this.user, timestamp + this.grantDelay, this.executionDelay, true);
+                    await expect(this.txResponse)
+                      .to.emit(this.manager, 'RoleGranted')
+                      .withArgs(ANOTHER_ROLE, this.user, this.executionDelay, timestamp + this.grantDelay, true);
 
                     // Access is correctly stored
                     const access = await this.manager.getAccess(ANOTHER_ROLE, this.user);
@@ -1278,7 +1278,7 @@ describe('AccessManager', function () {
                   .connect(this.admin)
                   .grantRole(ANOTHER_ROLE, this.user, executionDelay);
                 const grantedAt = await time.clockFromReceipt.timestamp(txResponse);
-                expect(txResponse)
+                await expect(txResponse)
                   .to.emit(this.manager, 'RoleGranted')
                   .withArgs(ANOTHER_ROLE, this.user, executionDelay, grantedAt, true);
 
@@ -1335,9 +1335,9 @@ describe('AccessManager', function () {
                     .grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay);
                   const timestamp = await time.clockFromReceipt.timestamp(txResponse);
 
-                  expect(txResponse)
+                  await expect(txResponse)
                     .to.emit(this.manager, 'RoleGranted')
-                    .withArgs(ANOTHER_ROLE, this.user, timestamp, this.newExecutionDelay, false);
+                    .withArgs(ANOTHER_ROLE, this.user, this.newExecutionDelay, timestamp, false);
 
                   // Access is correctly stored
                   const access = await this.manager.getAccess(ANOTHER_ROLE, this.user);
@@ -1370,10 +1370,10 @@ describe('AccessManager', function () {
                   this.delay = this.previousExecutionDelay - this.newExecutionDelay; // For testAsDelay
                 });
 
-                it('emits event', function () {
-                  expect(this.txResponse)
+                it('emits event', async function () {
+                  await expect(this.txResponse)
                     .to.emit(this.manager, 'RoleGranted')
-                    .withArgs(ANOTHER_ROLE, this.user, this.grantTimestamp + this.delay, this.newExecutionDelay, false);
+                    .withArgs(ANOTHER_ROLE, this.user, this.newExecutionDelay, this.grantTimestamp + this.delay, false);
                 });
 
                 testAsDelay('execution delay effect', {
@@ -1446,9 +1446,9 @@ describe('AccessManager', function () {
                     .grantRole(ANOTHER_ROLE, this.user, this.newExecutionDelay);
                   const timestamp = await time.clockFromReceipt.timestamp(txResponse);
 
-                  expect(txResponse)
+                  await expect(txResponse)
                     .to.emit(this.manager, 'RoleGranted')
-                    .withArgs(ANOTHER_ROLE, this.user, timestamp, this.newExecutionDelay, false);
+                    .withArgs(ANOTHER_ROLE, this.user, this.newExecutionDelay, timestamp, false);
 
                   // Access is correctly stored
                   const access = await this.manager.getAccess(ANOTHER_ROLE, this.user);
@@ -1481,10 +1481,10 @@ describe('AccessManager', function () {
                   this.delay = this.previousExecutionDelay - this.newExecutionDelay; // For testAsDelay
                 });
 
-                it('emits event', function () {
-                  expect(this.txResponse)
+                it('emits event', async function () {
+                  await expect(this.txResponse)
                     .to.emit(this.manager, 'RoleGranted')
-                    .withArgs(ANOTHER_ROLE, this.user, this.grantTimestamp + this.delay, this.newExecutionDelay, false);
+                    .withArgs(ANOTHER_ROLE, this.user, this.newExecutionDelay, this.grantTimestamp + this.delay, false);
                 });
 
                 testAsDelay('execution delay effect', {
@@ -1871,9 +1871,9 @@ describe('AccessManager', function () {
       const txResponse = await schedule();
 
       expect(await this.manager.getSchedule(operationId)).to.equal(scheduledAt + this.delay);
-      expect(txResponse)
+      await expect(txResponse)
         .to.emit(this.manager, 'OperationScheduled')
-        .withArgs(operationId, '1', scheduledAt + this.delay, this.target, this.calldata);
+        .withArgs(operationId, '1', scheduledAt + this.delay, this.caller, this.target, this.calldata);
     });
 
     it('schedules an operation at the minimum execution date if no specified execution date (when == 0)', async function () {
@@ -1886,9 +1886,9 @@ describe('AccessManager', function () {
       const operationId = await this.manager.hashOperation(this.caller, this.target, this.calldata);
 
       expect(await this.manager.getSchedule(operationId)).to.equal(scheduledAt + executionDelay);
-      expect(txResponse)
+      await expect(txResponse)
         .to.emit(this.manager, 'OperationScheduled')
-        .withArgs(operationId, '1', scheduledAt + executionDelay, this.target, this.calldata);
+        .withArgs(operationId, '1', scheduledAt + executionDelay, this.caller, this.target, this.calldata);
     });
 
     it('increases the nonce of an operation scheduled more than once', async function () {

test/governance/TimelockController.test.js

@@ -171,7 +171,7 @@ describe('TimelockController', function () {
               MINDELAY,
             );
 
-          expect(tx)
+          await expect(tx)
             .to.emit(this.mock, 'CallScheduled')
             .withArgs(
               this.operation.id,
@@ -698,7 +698,7 @@ describe('TimelockController', function () {
                   this.operation.salt,
                 );
               for (const i in this.operation.targets) {
-                expect(tx)
+                await expect(tx)
                   .to.emit(this.mock, 'CallExecuted')
                   .withArgs(
                     this.operation.id,
@@ -843,7 +843,7 @@ describe('TimelockController', function () {
                   nonReentrantBatchOperation.salt,
                 );
               for (const i in nonReentrantBatchOperation.targets) {
-                expect(tx)
+                await expect(tx)
                   .to.emit(this.mock, 'CallExecuted')
                   .withArgs(
                     nonReentrantBatchOperation.id,

test/governance/extensions/GovernorTimelockAccess.test.js

@@ -370,7 +370,7 @@ describe('GovernorTimelockAccess', function () {
             if (await this.mock.proposalNeedsQueuing(this.proposal.id)) {
               expect(await this.helper.queue())
                 .to.emit(this.mock, 'ProposalQueued')
-                .withArgs(this.proposal.id);
+                .withArgs(this.proposal.id, anyValue);
             }
             if (delay > 0) {
               await this.helper.waitForEta();
@@ -378,7 +378,7 @@ describe('GovernorTimelockAccess', function () {
             expect(await this.helper.execute())
               .to.emit(this.mock, 'ProposalExecuted')
               .withArgs(this.proposal.id)
-              .to.not.emit(this.receiver, 'CalledUnrestricted');
+              .to.emit(this.receiver, 'CalledUnrestricted');
           });
         }
       });

test/proxy/Proxy.behaviour.js

@@ -6,8 +6,9 @@ const { getAddressInSlot, ImplementationSlot } = require('../helpers/storage');
 module.exports = function shouldBehaveLikeProxy() {
   it('cannot be initialized with a non-contract address', async function () {
     const initializeData = '0x';
+    const contractFactory = await ethers.getContractFactory('ERC1967Proxy');
     await expect(this.createProxy(this.nonContractAddress, initializeData))
-      .to.be.revertedWithCustomError(await ethers.getContractFactory('ERC1967Proxy'), 'ERC1967InvalidImplementation')
+      .to.be.revertedWithCustomError(contractFactory, 'ERC1967InvalidImplementation')
       .withArgs(this.nonContractAddress);
   });
 

test/proxy/beacon/BeaconProxy.test.js

@@ -22,7 +22,7 @@ describe('BeaconProxy', function () {
     Object.assign(this, await loadFixture(fixture));
   });
 
-  describe('bad beacon is not accepted', async function () {
+  describe('bad beacon is not accepted', function () {
     it('non-contract beacon', async function () {
       const notBeacon = this.other;
 
@@ -34,7 +34,9 @@ describe('BeaconProxy', function () {
     it('non-compliant beacon', async function () {
       const badBeacon = await ethers.deployContract('BadBeaconNoImpl');
 
-      await expect(this.newBeaconProxy(badBeacon, '0x')).to.be.revertedWithoutReason;
+      // BadBeaconNoImpl does not provide `implementation()` has no fallback.
+      // This causes ERC1967Utils._setBeacon to revert.
+      await expect(this.newBeaconProxy(badBeacon, '0x')).to.be.revertedWithoutReason();
     });
 
     it('non-contract implementation', async function () {
@@ -92,7 +94,7 @@ describe('BeaconProxy', function () {
     });
   });
 
-  describe('upgrade', async function () {
+  describe('upgrade', function () {
     it('upgrade a proxy by upgrading its beacon', async function () {
       const value = 10n;
       const data = this.v1.interface.encodeFunctionData('initializeNonPayableWithValue', [value]);

test/proxy/beacon/UpgradeableBeacon.test.js

@@ -23,7 +23,7 @@ describe('UpgradeableBeacon', function () {
       .withArgs(this.other);
   });
 
-  describe('once deployed', async function () {
+  describe('once deployed', function () {
     it('emits Upgraded event to the first implementation', async function () {
       await expect(this.beacon.deploymentTransaction()).to.emit(this.beacon, 'Upgraded').withArgs(this.v1);
     });

test/token/ERC1155/ERC1155.behavior.js

@@ -173,7 +173,7 @@ function shouldBehaveLikeERC1155() {
         });
       }
 
-      describe('when called by the holder', async function () {
+      describe('when called by the holder', function () {
         beforeEach(async function () {
           this.args = {
             operator: this.holder,
@@ -490,7 +490,7 @@ function shouldBehaveLikeERC1155() {
         });
       }
 
-      describe('when called by the holder', async function () {
+      describe('when called by the holder', function () {
         beforeEach(async function () {
           this.args = {
             operator: this.holder,

test/token/ERC1155/extensions/ERC1155Supply.test.js

@@ -111,7 +111,7 @@ describe('ERC1155Supply', function () {
 
   describe('other', function () {
     it('supply unaffected by no-op', async function () {
-      this.token.safeTransferFrom(ethers.ZeroAddress, ethers.ZeroAddress, firstTokenId, firstTokenValue, '0x');
+      await this.token.$_update(ethers.ZeroAddress, ethers.ZeroAddress, [firstTokenId], [firstTokenValue]);
       expect(await this.token.totalSupply(ethers.Typed.uint256(firstTokenId))).to.equal(0n);
       expect(await this.token.totalSupply()).to.equal(0n);
     });

test/token/ERC20/extensions/ERC4626.test.js

@@ -46,7 +46,7 @@ describe('ERC4626', function () {
     }
   });
 
-  describe('reentrancy', async function () {
+  describe('reentrancy', function () {
     const reenterType = Enum('No', 'Before', 'After');
 
     const value = 1_000_000_000_000_000_000n;
@@ -182,7 +182,7 @@ describe('ERC4626', function () {
     });
   });
 
-  describe('limits', async function () {
+  describe('limits', function () {
     beforeEach(async function () {
       const token = await ethers.deployContract('$ERC20DecimalsMock', [name, symbol, decimals]);
       const vault = await ethers.deployContract('$ERC4626LimitsMock', ['', '', token]);

test/token/ERC721/ERC721.behavior.js

@@ -653,8 +653,8 @@ function shouldBehaveLikeERC721() {
       });
     });
 
-    describe('getApproved', async function () {
-      describe('when token is not minted', async function () {
+    describe('getApproved', function () {
+      describe('when token is not minted', function () {
         it('reverts', async function () {
           await expect(this.token.getApproved(nonExistentTokenId))
             .to.be.revertedWithCustomError(this.token, 'ERC721NonexistentToken')
@@ -662,12 +662,12 @@ function shouldBehaveLikeERC721() {
         });
       });
 
-      describe('when token has been minted ', async function () {
+      describe('when token has been minted ', function () {
         it('should return the zero address', async function () {
           expect(await this.token.getApproved(firstTokenId)).to.equal(ethers.ZeroAddress);
         });
 
-        describe('when account has been approved', async function () {
+        describe('when account has been approved', function () {
           beforeEach(async function () {
             await this.token.connect(this.owner).approve(this.approved, firstTokenId);
           });
@@ -687,7 +687,7 @@ function shouldBehaveLikeERC721() {
         .withArgs(ethers.ZeroAddress);
     });
 
-    describe('with minted token', async function () {
+    describe('with minted token', function () {
       beforeEach(async function () {
         this.tx = await this.token.$_mint(this.owner, firstTokenId);
       });
@@ -856,7 +856,7 @@ function shouldBehaveLikeERC721Enumerable() {
         .withArgs(ethers.ZeroAddress);
     });
 
-    describe('with minted token', async function () {
+    describe('with minted token', function () {
       beforeEach(async function () {
         await this.token.$_mint(this.owner, firstTokenId);
       });