From b06baa93a195f4f4b55ef4edf1a68a9ce875c327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kaan=20Uzdo=C4=9Fan?= Date: Thu, 30 Jul 2026 11:38:14 +0300 Subject: [PATCH] fix: make the new-chain guard a real test so CI fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "at least one passing test" guard lived in a suite-level after() hook. A throwing hook is reported as a failed suite but leaves the failure count at 0, and Node >=22.22 (CI uses node-version: "22") derives its exit code from the counters — so the runner exited 0 and the job went green on a run where the guard had fired. Node 20.x and 22.5 still exit 1, which is why this went unnoticed. Moving the check into a real test registered after the per-chain loop makes it exit non-zero again, and makes it show up as a failing testcase in the junit report instead of disappearing from it. Co-Authored-By: Claude Opus 4.8 --- tests/chain-tests.test.ts | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/tests/chain-tests.test.ts b/tests/chain-tests.test.ts index 506f000..b202c90 100644 --- a/tests/chain-tests.test.ts +++ b/tests/chain-tests.test.ts @@ -1,4 +1,4 @@ -import { describe, it, before, after } from "node:test"; +import { describe, it, before } from "node:test"; import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import { resolve, dirname } from "node:path"; @@ -212,16 +212,6 @@ describe("Test Supported Chains", { timeout: TEST_TIME }, () => { let anyTestsPass = false; - after(() => { - if (!anyTestsPass && newAddedChainIds.length) { - throw new Error( - "There needs to be at least one passing test. Did you forget to add a test for your new chain with the id(s) " + - newAddedChainIds.join(",") + - "?", - ); - } - }); - const testedChains = new Set(); for (const chainId of chainsToTest) { @@ -274,6 +264,23 @@ describe("Test Supported Chains", { timeout: TEST_TIME }, () => { }); } + // Registered after the per-chain tests above so `anyTestsPass` is settled by + // the time it runs. This has to be a real test, not an `after()` hook: a + // throwing suite-level hook is reported but leaves the failure count at 0, and + // Node >=22.22 exits 0 in that case, so CI would go green on a failed run. + it( + "should have run at least one test for the new chain(s)", + { skip: newAddedChainIds.length === 0 }, + () => { + assert.ok( + anyTestsPass, + "There needs to be at least one passing test. Did you forget to add a test for your new chain with the id(s) " + + newAddedChainIds.join(",") + + "?", + ); + }, + ); + it("should have included Etherscan contracts for all testedChains having etherscanAPI", () => { const missingEtherscanTests: string[] = []; for (const chainId of chainsToTest) {