From fe9ec27f24229a0898182d6ab1eccadbc5c53074 Mon Sep 17 00:00:00 2001 From: Bryant Austin Date: Wed, 22 Jul 2026 15:30:40 -0600 Subject: [PATCH] Fix error-expecting test handling in CLI and server runners --- src/server/test-execution-service.ts | 10 +++++++--- src/services/test-runner.ts | 20 ++++++++++++++++---- src/shared/results-shared.ts | 25 ++++++++++++++++++++++++- test/run-tests.test.ts | 3 ++- 4 files changed, 49 insertions(+), 9 deletions(-) diff --git a/src/server/test-execution-service.ts b/src/server/test-execution-service.ts index cfdb94a..f975c44 100644 --- a/src/server/test-execution-service.ts +++ b/src/server/test-execution-service.ts @@ -7,6 +7,7 @@ import { TestLoader } from '../loaders/test-loader.js'; import { generateEmptyResults, generateParametersResource, + responseIndicatesError, Result, } from '../shared/results-shared.js'; import { InternalTestResult, Tests } from '../models/test-types.js'; @@ -103,16 +104,19 @@ export class TestExecutionService { result.responseStatus = response.status; const responseBody = await response.json(); - const parsedExpected = cvl.parse(result.expected); + const parsedExpected = + result.expected !== undefined ? cvl.parse(result.expected) : undefined; result.actual = resultExtractor.extract(responseBody, { singletonListKeys: ValueMap.singletonListKeysFromExpected(parsedExpected), }); const invalid = result.invalid; + const erroredOut = responseIndicatesError(response.status, responseBody); if (invalid === 'true' || invalid === 'semantic') { - result.testStatus = response.status === 200 ? 'fail' : 'pass'; + // The expression is expected to error; it passes only if the engine erred. + result.testStatus = erroredOut ? 'pass' : 'fail'; } else { - if (response.status === 200) { + if (!erroredOut) { result.testStatus = resultsEqual(parsedExpected, result.actual) ? 'pass' : 'fail'; } else { result.testStatus = 'fail'; diff --git a/src/services/test-runner.ts b/src/services/test-runner.ts index b68627d..6a3d5cb 100644 --- a/src/services/test-runner.ts +++ b/src/services/test-runner.ts @@ -2,7 +2,11 @@ import { ConfigLoader } from '../conf/config-loader.js'; import { CQLEngine } from '../cql-engine/cql-engine.js'; import { TestLoader } from '../loaders/test-loader.js'; import { CQLTestResults } from '../test-results/cql-test-results.js'; -import { generateEmptyResults, generateParametersResource } from '../shared/results-shared.js'; +import { + generateEmptyResults, + generateParametersResource, + responseIndicatesError, +} from '../shared/results-shared.js'; import { InternalTestResult } from '../models/test-types.js'; import { ResultExtractor } from '../extractors/result-extractor.js'; import { ServerConnectivity } from '../shared/server-connectivity.js'; @@ -165,6 +169,11 @@ export class TestRunner { headers: { 'Content-Type': 'application/json', }, + // Do not throw on non-2xx: a non-2xx status is a valid engine + // response about the expression (e.g. an OperationOutcome), which the + // classification logic below must see so that invalid tests can pass + // and valid tests fail — matching the fetch path. See responseIndicatesError. + validateStatus: () => true, }); response = { status: axiosResponse.status, @@ -187,16 +196,19 @@ export class TestRunner { result.responseStatus = response.status; const responseBody = response.data; - const parsedExpected = cvl.parse(result.expected); + const parsedExpected = + result.expected !== undefined ? cvl.parse(result.expected) : undefined; result.actual = resultExtractor.extract(responseBody, { singletonListKeys: ValueMap.singletonListKeysFromExpected(parsedExpected), }); const invalid = result.invalid; + const erroredOut = responseIndicatesError(response.status, responseBody); if (invalid === 'true' || invalid === 'semantic') { - result.testStatus = response.status === 200 ? 'fail' : 'pass'; + // The expression is expected to error; it passes only if the engine erred. + result.testStatus = erroredOut ? 'pass' : 'fail'; } else { - if (response.status === 200) { + if (!erroredOut) { result.testStatus = resultsEqual(parsedExpected, result.actual) ? 'pass' : 'fail'; diff --git a/src/shared/results-shared.ts b/src/shared/results-shared.ts index b910d55..04973a5 100644 --- a/src/shared/results-shared.ts +++ b/src/shared/results-shared.ts @@ -48,7 +48,10 @@ export class Result implements InternalTestResult { } else { this.expected = test.output as string; } - } else { + } else if (this.invalid !== 'true' && this.invalid !== 'semantic') { + // No output is expected only when the expression is marked invalid ("true" + // for a run-time error, "semantic" for a translation error) — the test expects + // an error. Otherwise there is nothing to compare against, so skip. this.testStatus = 'skip'; this.skipMessage = 'No output specified'; } @@ -100,6 +103,26 @@ export async function generateEmptyResults( return groupResults; } +/** + * Determines whether a CQL evaluation response represents an error. Used to decide + * whether `invalid="true"`/`invalid="semantic"` tests pass (an error is expected). + * + * The engine does not always signal a run-time error with a non-2xx HTTP status: the + * FHIR `$cql`/`$evaluate` operations typically return HTTP 200 with a `Parameters` + * resource carrying an `evaluation error` parameter (an OperationOutcome). We treat + * both a non-2xx status and the presence of that parameter as an error. + */ +export function responseIndicatesError(status: number | undefined, responseBody: any): boolean { + if (status !== undefined && (status < 200 || status >= 300)) { + return true; + } + const parameters = responseBody?.parameter; + if (Array.isArray(parameters)) { + return parameters.some((p: any) => p?.name === 'evaluation error'); + } + return false; +} + export function generateParametersResource( result: InternalTestResult, cqlEndpoint: string diff --git a/test/run-tests.test.ts b/test/run-tests.test.ts index 4f0d7a2..4932db5 100644 --- a/test/run-tests.test.ts +++ b/test/run-tests.test.ts @@ -16,7 +16,8 @@ const makeResult = (testsName: string, groupName: string, testName: string) => ( capability: [], }); -vi.mock('../src/shared/results-shared', () => ({ +vi.mock('../src/shared/results-shared', async orig => ({ + ...(await orig()), generateEmptyResults: vi .fn() .mockImplementation(async () => [