From 5228b01316bc17150ec74d6d18820c4bccd94268 Mon Sep 17 00:00:00 2001 From: Benjamin Van Dam Date: Fri, 3 Jul 2026 15:10:04 +0200 Subject: [PATCH] Cap the liquid-test polling interval at 5 seconds The delay between result polls grows 5% per poll with no upper bound, so a long test run (~10 minutes server-side) ends up being checked only every 30-60 seconds - a finished run can sit unnoticed for up to a minute, and in CI that lag multiplies across batches. Cap the delay at 5s: at most one poll per 5 seconds per run (reached after ~70s), bounding the detection lag without meaningfully increasing request volume for short runs. --- CHANGELOG.md | 3 +++ lib/liquidTestRunner.js | 6 ++++- package-lock.json | 4 ++-- package.json | 2 +- tests/TESTS.md | 1 + tests/lib/liquidTestRunner.test.js | 35 ++++++++++++++++++++++++++++++ 6 files changed, 47 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee8d0f3..abd8ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ All notable changes to this project will be documented in this file. +## [1.56.4] (03/07/2026) +Cap the liquid-test polling interval at 5 seconds. The delay between result polls grew 5% per poll without a limit, so long test runs (~10 minutes) were only checked every 30-60 seconds and a finished run could sit unnoticed for up to a minute. + ## [1.56.3] (03/07/2026) Improve `run-test --status` output for CI: surface the underlying error message when a run ends in `test_error`/`internal_error` (previously reported as a bare `FAILED` with no reason), and suppress the progress spinner when stdout is not a TTY (it flooded CI logs with hundreds of "Running tests.." frames). diff --git a/lib/liquidTestRunner.js b/lib/liquidTestRunner.js index d71da85..985e961 100644 --- a/lib/liquidTestRunner.js +++ b/lib/liquidTestRunner.js @@ -311,6 +311,10 @@ function buildTestParams(firmId, templateType, handle, testName = "", renderMode async function fetchResult(firmId, testRunId, templateType) { let testRun = { status: "started" }; let pollingDelay = 1000; + // Without a cap the 5% growth leaves long test runs polling every 30-60s (or more), + // so a finished run can sit unnoticed for up to a minute. Capping keeps the request + // rate modest (at most one poll per 5s per run) while bounding the detection lag. + const maxPollingDelay = 5000; const waitingLimit = 2000000; spinner.spin("Running tests.."); @@ -322,7 +326,7 @@ async function fetchResult(firmId, testRunId, templateType) { const response = await SF.readTestRun(firmId, testRunId, templateType); testRun = response.data; waitingTime += pollingDelay; - pollingDelay *= 1.05; + pollingDelay = Math.min(pollingDelay * 1.05, maxPollingDelay); if (waitingTime >= waitingLimit) { spinner.stop(); consola.error("Timeout. Try to run your test again"); diff --git a/package-lock.json b/package-lock.json index 2f2f721..0137fb4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "silverfin-cli", - "version": "1.56.3", + "version": "1.56.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "silverfin-cli", - "version": "1.56.3", + "version": "1.56.4", "license": "MIT", "dependencies": { "axios": "^1.6.2", diff --git a/package.json b/package.json index ce4d566..ebfa431 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "silverfin-cli", - "version": "1.56.3", + "version": "1.56.4", "description": "Command line tool for Silverfin template development", "main": "index.js", "license": "MIT", diff --git a/tests/TESTS.md b/tests/TESTS.md index 20024bd..824d550 100644 --- a/tests/TESTS.md +++ b/tests/TESTS.md @@ -857,6 +857,7 @@ Source: `lib/liquidTestRunner.js` | `runTests` | should return undefined when config is missing | Verifies that `undefined` is returned and an error is logged when the template's `config.json` does not exist. | | `runTests` | should return undefined when YAML test file does not exist | Verifies that `undefined` is returned and an error is logged when the YAML test file path in the config does not exist on disk. | | `runTests` | should run tests and return testRun result when YAML exists and API responds | Verifies that `createTestRun` and `readTestRun` are called and the completed test-run object is returned. | +| `runTests` | should cap the polling interval at 5 seconds on long test runs | Verifies that the poll delay growth is capped at 5s, so a long-running test is still detected promptly once it finishes. | | `runTests` | should log info and return false when YAML file is empty (single line) | Verifies that `undefined` is returned and an info message about no stored tests is logged when the YAML file contains only a comment. | | `runTests` | should run tests for accountTemplate type | Verifies that `runTests` works end-to-end for the `accountTemplate` type and returns a completed test-run object. | | `runTestsWithOutput` | should exit with error for invalid templateType | Verifies that an invalid `templateType` causes an error to be logged and `process.exit(1)` to be called. | diff --git a/tests/lib/liquidTestRunner.test.js b/tests/lib/liquidTestRunner.test.js index 68674bc..52e849a 100644 --- a/tests/lib/liquidTestRunner.test.js +++ b/tests/lib/liquidTestRunner.test.js @@ -219,6 +219,41 @@ describe("runTests", () => { expect(result.testRun.tests).toEqual(makePassedTests()); }); + it("should cap the polling interval at 5 seconds on long test runs", async () => { + const handle = "reconciliation_text_1"; + setupFsUtilsMocks(handle); + + const testDir = path.join(tempDir, "reconciliation_texts", handle, "tests"); + fs.mkdirSync(testDir, { recursive: true }); + fs.writeFileSync(path.join(testDir, `${handle}_liquid_test.yml`), SIMPLE_YAML); + + ReconciliationText.read.mockReturnValue({ + handle, + text: "{% assign x = 1 %}", + text_parts: [], + }); + + SF.createTestRun = jest.fn().mockResolvedValue({ data: 42 }); + // Keep the run "started" for 60 polls - enough for the uncapped 5% growth to pass + // 5s (1000 * 1.05^33 ≈ 5000) - then complete. + let polls = 0; + SF.readTestRun = jest.fn().mockImplementation(async () => { + polls += 1; + return { data: makeTestRun(polls < 60 ? "started" : "completed", makePassedTests()) }; + }); + const setTimeoutSpy = jest.spyOn(global, "setTimeout"); + + const promise = runTests(1001, "reconciliationText", handle, "", false, "none"); + await jest.runAllTimersAsync(); + const result = await promise; + + expect(result.testRun.status).toBe("completed"); + const delays = setTimeoutSpy.mock.calls.map((call) => call[1]).filter((delay) => typeof delay === "number"); + // The cap must actually be reached, and never exceeded. + expect(Math.max(...delays)).toBe(5000); + setTimeoutSpy.mockRestore(); + }); + it("should log info and return undefined when YAML file is empty (single line)", async () => { const handle = "reconciliation_text_1"; setupFsUtilsMocks(handle);