fix: address PR comments and improve import consistency

- Convert require() statements to ES6 imports where possible
- Improve type safety in test utility functions
- Export previously unused functions in issuanceCalculations.ts
- Standardize import organization across test files
- Fix mixed import styles in test files
- Organize imports properly in commonTestUtils.ts

All tests pass (161/161) and linting is clean.

Author: RembrandtK

packages/issuance/test/tests/DirectAllocation.test.ts

@@ -1,6 +1,8 @@
 import { expect } from 'chai'
 import hre from 'hardhat'
+
 const { ethers } = hre
+
 const { upgrades } = require('hardhat')
 
 import { deployDirectAllocation, deployTestGraphToken, getTestAccounts, SHARED_CONSTANTS } from './helpers/fixtures'

packages/issuance/test/tests/IssuanceSystem.test.ts

@@ -3,10 +3,10 @@
  * Reduced from 149 lines to ~80 lines using shared utilities
  */
 
-const { expect } = require('chai')
+import { expect } from 'chai'
 
-const { setupOptimizedIssuanceSystem } = require('../utils/optimizedFixtures')
-const { TestConstants, mineBlocks, expectRatioToEqual } = require('../utils/testPatterns')
+import { setupOptimizedIssuanceSystem } from '../utils/optimizedFixtures'
+import { expectRatioToEqual, mineBlocks, TestConstants } from '../utils/testPatterns'
 
 describe('Issuance System', () => {
   let system: any

packages/issuance/test/tests/RewardsEligibilityOracle.test.ts

@@ -3,6 +3,7 @@ import '@nomicfoundation/hardhat-chai-matchers'
 import { time } from '@nomicfoundation/hardhat-network-helpers'
 import { expect } from 'chai'
 import hre from 'hardhat'
+
 const { ethers } = hre
 const { upgrades } = require('hardhat')
 

packages/issuance/test/tests/consolidated/InterfaceCompliance.test.ts

@@ -1,10 +1,9 @@
-import { expect } from 'chai'
-const { ethers } = require('hardhat')
-
-const { shouldSupportERC165Interface } = require('../../utils/testPatterns')
 // Import generated interface IDs from the interfaces package
 import { IIssuanceAllocator, IIssuanceTarget, IRewardsEligibilityOracle } from '@graphprotocol/interfaces'
+import { expect } from 'chai'
+import { ethers } from 'hardhat'
 
+import { shouldSupportERC165Interface } from '../../utils/testPatterns'
 import {
   deployDirectAllocation,
   deployIssuanceAllocator,

packages/issuance/test/tests/helpers/commonTestUtils.ts

@@ -4,14 +4,15 @@
 
 import type { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers'
 import { expect } from 'chai'
+import type { Contract } from 'ethers'
+
 /**
  * Test multiple access control methods on a contract
  * @param contract - The contract to test
  * @param methods - Array of methods to test with their arguments
  * @param authorizedAccount - Account that should have access
  * @param unauthorizedAccount - Account that should not have access
  */
-import type { Contract } from 'ethers'
 
 export async function testMultipleAccessControl(
   contract: Contract,

packages/issuance/test/tests/helpers/fixtures.ts

@@ -1,8 +1,10 @@
+import '@nomicfoundation/hardhat-chai-matchers'
+
 import fs from 'fs'
 import hre from 'hardhat'
+
 const { ethers } = hre
 const { upgrades } = require('hardhat')
-import '@nomicfoundation/hardhat-chai-matchers'
 
 import type { SignerWithAddress } from '@nomicfoundation/hardhat-ethers/signers'
 

packages/issuance/test/utils/issuanceCalculations.ts

@@ -1,4 +1,4 @@
-const { ethers } = require('hardhat')
+import { ethers } from 'hardhat'
 
 /**
  * Shared calculation utilities for issuance tests.
@@ -156,7 +156,7 @@ export function parseEther(value: string): bigint {
 /**
  * Helper to format wei bigint to ETH string for debugging.
  */
-function formatEther(value: bigint): string {
+export function formatEther(value: bigint): string {
   return ethers.formatEther(value)
 }
 
@@ -168,17 +168,6 @@ function formatEther(value: bigint): string {
  * @param endBlock - Ending block number
  * @returns Number of blocks for accumulation calculation
  */
-function calculateBlockDifference(startBlock: number, endBlock: number): bigint {
+export function calculateBlockDifference(startBlock: number, endBlock: number): bigint {
   return BigInt(Math.max(0, endBlock - startBlock))
 }
-
-module.exports = {
-  calculateExpectedAccumulation,
-  calculateProportionalDistribution,
-  calculateExpectedTargetIssuance,
-  calculateMultiTargetIssuance,
-  verifyTotalDistribution,
-  parseEther,
-  formatEther,
-  calculateBlockDifference,
-}

packages/issuance/test/utils/testPatterns.ts

@@ -2,8 +2,8 @@
  * Shared test patterns and utilities to reduce duplication across test files
  */
 
-const { expect } = require('chai')
-const { ethers } = require('hardhat')
+import { expect } from 'chai'
+import { ethers } from 'hardhat'
 
 // Type definitions for test utilities
 export interface TestAccounts {
@@ -71,7 +71,7 @@ export function shouldEnforceGovernorRole<T>(
 
       await expect(
         (contract as any).connect(testAccounts.nonGovernor)[methodName](...methodArgs),
-      ).to.be.revertedWithCustomError(contract, 'AccessControlUnauthorizedAccount')
+      ).to.be.revertedWithCustomError(contract as any, 'AccessControlUnauthorizedAccount')
     })
 
     it(`should allow governor to call ${methodName}`, async function () {
@@ -100,7 +100,7 @@ export function shouldEnforceRoleAccess<T>(
 
       await expect(
         (contract as any).connect(testAccounts.nonGovernor)[methodName](...methodArgs),
-      ).to.be.revertedWithCustomError(contract, 'AccessControlUnauthorizedAccount')
+      ).to.be.revertedWithCustomError(contract as any, 'AccessControlUnauthorizedAccount')
     })
   }
 }
@@ -161,6 +161,7 @@ export function shouldInitializeCorrectly<T>(contractGetter: () => T, expectedVa
     Object.entries(expectedValues).forEach(([property, expectedValue]) => {
       it(`should set ${property} correctly during initialization`, async function () {
         const contract = contractGetter()
+        // Type assertion is necessary here since we're accessing dynamic properties
         const actualValue = await (contract as any)[property]()
         expect(actualValue).to.equal(expectedValue)
       })
@@ -171,7 +172,7 @@ export function shouldInitializeCorrectly<T>(contractGetter: () => T, expectedVa
       const accounts = this.parent.ctx.accounts
 
       await expect((contract as any).initialize(accounts.governor.address)).to.be.revertedWithCustomError(
-        contract,
+        contract as any,
         'InvalidInitialization',
       )
     })
@@ -278,7 +279,7 @@ export function shouldEnforceAccessControl<T>(
           const contract = contractGetter()
           await expect(
             (contract as any).connect(accounts.nonGovernor)[method.name](...method.args),
-          ).to.be.revertedWithCustomError(contract, 'AccessControlUnauthorizedAccount')
+          ).to.be.revertedWithCustomError(contract as any, 'AccessControlUnauthorizedAccount')
         })
 
         allowedRoles.forEach((role) => {
@@ -335,7 +336,7 @@ export function shouldInitializeProperly<T>(
           const contract = contractGetter()
           await expect(
             (contract as any)[reinitializationTest.method](...reinitializationTest.args),
-          ).to.be.revertedWithCustomError(contract, reinitializationTest.expectedError)
+          ).to.be.revertedWithCustomError(contract as any, reinitializationTest.expectedError)
         })
       }
     })
@@ -377,7 +378,7 @@ export function shouldHandlePausability<T>(
       it('should revert when non-PAUSE_ROLE tries to pause', async function () {
         const contract = contractGetter()
         await expect((contract as any).connect(accounts.nonGovernor).pause()).to.be.revertedWithCustomError(
-          contract,
+          contract as any,
           'AccessControlUnauthorizedAccount',
         )
       })
@@ -400,7 +401,7 @@ export function shouldHandlePausability<T>(
 
           await expect(
             (contract as any).connect(caller)[operation.name](...operation.args),
-          ).to.be.revertedWithCustomError(contract, 'EnforcedPause')
+          ).to.be.revertedWithCustomError(contract as any, 'EnforcedPause')
         })
       })
     })
@@ -455,7 +456,7 @@ export function shouldManageRoles<T>(
             const contract = contractGetter()
             await expect(
               (contract as any).connect(accounts.nonGovernor).grantRole(roleConfig.role, accounts.user.address),
-            ).to.be.revertedWithCustomError(contract, 'AccessControlUnauthorizedAccount')
+            ).to.be.revertedWithCustomError(contract as any, 'AccessControlUnauthorizedAccount')
           })
         })
       })
@@ -520,7 +521,7 @@ export function shouldValidateInputs<T>(
             test.caller === 'operator' ? accounts.operator : test.caller === 'user' ? accounts.user : accounts.governor
 
           await expect((contract as any).connect(caller)[test.method](...test.args)).to.be.revertedWithCustomError(
-            contract,
+            contract as any,
             test.expectedError,
           )
         })