Skip to content

Commit f8233f3

Browse files
committed
refactor: address PR review feedback
• Convert require() to ES6 imports for consistency (chai, ethers) • Add descriptive comments to empty catch blocks in generateInterfaceIds.js • Add mocha configuration to hardhat.config.cjs • Remove commented-out paths configuration • Add test:coverage script to package.json • Improve error handling in fixtures.ts (console.warn → console.error + throw)
1 parent 13f0ae0 commit f8233f3

File tree

7 files changed

+22
-15
lines changed

7 files changed

+22
-15
lines changed

packages/issuance/hardhat.config.cjs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,6 @@ const dotenv = require('dotenv')
1313
dotenv.config()
1414

1515
const config = {
16-
// paths: {
17-
// sources: './contracts',
18-
// tests: './test',
19-
// artifacts: './build/contracts',
20-
// cache: './cache',
21-
// },
2216
solidity: {
2317
compilers: [
2418
{
@@ -85,6 +79,10 @@ const config = {
8579
currency: 'USD',
8680
outputFile: 'reports/gas-report.log',
8781
},
82+
mocha: {
83+
reporter: 'spec',
84+
timeout: 60000,
85+
},
8886
typechain: {
8987
outDir: 'types',
9088
target: 'ethers-v6',

packages/issuance/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
"clean": "rm -rf build/ cache/ dist/ forge-artifacts/ cache_forge/",
1717
"compile": "hardhat compile",
1818
"test": "pnpm --filter @graphprotocol/issuance-test test",
19+
"test:coverage": "pnpm --filter @graphprotocol/issuance-test run test:coverage",
1920
"lint": "pnpm lint:ts; pnpm lint:sol; pnpm lint:md; pnpm lint:json",
2021
"lint:ts": "eslint '**/*.{js,ts,cjs,mjs,jsx,tsx}' --fix --cache; prettier -w --cache --log-level warn '**/*.{js,ts,cjs,mjs,jsx,tsx}'",
2122
"lint:sol": "solhint --fix --noPrompt --noPoster 'contracts/**/*.sol'; prettier -w --cache --log-level warn 'contracts/**/*.sol'",

packages/issuance/test/scripts/generateInterfaceIds.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ main().catch((error) => {
6666
try {
6767
fs.unlinkSync(tempScript)
6868
} catch {
69-
// Ignore cleanup errors
69+
// Ignore cleanup errors - temp file may not exist
7070
}
7171

7272
if (code === 0) {
@@ -80,7 +80,7 @@ main().catch((error) => {
8080
return
8181
}
8282
} catch {
83-
// Not JSON, continue
83+
// Not JSON, continue - this is expected for non-JSON output lines
8484
}
8585
}
8686
reject(new Error('Could not parse interface IDs from output'))

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ import '@nomicfoundation/hardhat-chai-matchers'
22

33
import { time } from '@nomicfoundation/hardhat-network-helpers'
44
import { expect } from 'chai'
5+
import { ethers } from 'hardhat'
56

6-
const { ethers, upgrades } = require('hardhat')
7+
const { upgrades } = require('hardhat')
78

89
import type { IGraphToken, RewardsEligibilityOracle } from '../../types'
910
import {
@@ -75,7 +76,8 @@ describe('RewardsEligibilityOracle', () => {
7576
await rewardsEligibilityOracle.connect(accounts.governor).revokeRole(OPERATOR_ROLE, accounts.governor.address)
7677
}
7778
} catch {
78-
// Ignore role management errors during reset
79+
// Role management errors during reset are non-fatal and may occur if roles are already revoked or not present.
80+
// These errors are expected and can be safely ignored.
7981
}
8082

8183
// Reset to default values

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
* Tests access control patterns across all contracts to reduce duplication
55
*/
66

7-
const { expect } = require('chai')
8-
const { deploySharedContracts, resetContractState, SHARED_CONSTANTS } = require('../helpers/fixtures')
7+
import { expect } from 'chai'
8+
9+
import { deploySharedContracts, resetContractState, SHARED_CONSTANTS } from '../helpers/fixtures'
910

1011
describe('Consolidated Access Control Tests', () => {
1112
let accounts: any

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,9 @@
55

66
import type { HardhatEthersSigner } from '@nomicfoundation/hardhat-ethers/signers'
77
import * as fs from 'fs'
8+
import { ethers } from 'hardhat'
89

9-
const { ethers, upgrades } = require('hardhat')
10+
const { upgrades } = require('hardhat')
1011

1112
// Shared test constants
1213
export const SHARED_CONSTANTS = {
@@ -180,6 +181,10 @@ export async function resetContractState(contracts: SharedContracts, accounts: T
180181
await rewardsEligibilityOracle.connect(accounts.governor).setEligibilityValidation(false)
181182
}
182183
} catch (error) {
183-
console.warn('RewardsEligibilityOracle state reset failed:', error instanceof Error ? error.message : String(error))
184+
console.error(
185+
'RewardsEligibilityOracle state reset failed:',
186+
error instanceof Error ? error.message : String(error),
187+
)
188+
throw error
184189
}
185190
}

packages/issuance/test/utils/testPatterns.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Shared test patterns and utilities to reduce duplication across test files
44
*/
55

6-
const { expect } = require('chai')
6+
import { expect } from 'chai'
77

88
// Test constants - centralized to avoid magic numbers
99
export const TestConstants = {

0 commit comments

Comments
 (0)