diff --git a/assets/schema/cql-test-results.schema.json b/assets/schema/cql-test-results.schema.json index 8f8e0cd..e8943f0 100644 --- a/assets/schema/cql-test-results.schema.json +++ b/assets/schema/cql-test-results.schema.json @@ -71,7 +71,8 @@ }, "invalid": { "type": "string", - "enum": ["true", "false", "semantic"] + "description": "The test's expected outcome, per InvalidType in testSchema.xsd: false to evaluate successfully, or the kind of error expected.", + "enum": ["false", "syntax", "semantic", "execution", "true"] }, "capabilities": { "type": "array", diff --git a/src/commands/build-cql-command.ts b/src/commands/build-cql-command.ts index 57fbea4..c54eac0 100644 --- a/src/commands/build-cql-command.ts +++ b/src/commands/build-cql-command.ts @@ -4,6 +4,7 @@ import * as path from 'node:path'; import { ConfigLoader } from '../conf/config-loader.js'; import { TestLoader } from '../loaders/test-loader.js'; import { generateEmptyResults } from '../shared/results-shared.js'; +import { preventsTranslation } from '../shared/invalid-utils.js'; export class BuildCommand { async execute(options: any): Promise { @@ -50,7 +51,9 @@ export class BuildCommand { continue; } - if (r.invalid !== 'semantic') { + // A define that cannot be translated would fail the whole generated library, so + // expressions expected to produce a syntax or semantic error are left out. + if (!preventsTranslation(r.invalid)) { const defineVal = `define "${r.groupName}.${r.testName}": ${r.expression}`; const key = `${r.testsName}-${r.groupName}-${r.testName}`; let reason = ''; diff --git a/src/models/test-types.ts b/src/models/test-types.ts index 7fc06ed..36e71f3 100644 --- a/src/models/test-types.ts +++ b/src/models/test-types.ts @@ -1,11 +1,13 @@ +import type { InvalidKind, ResultInvalidKind } from '../shared/invalid-utils.js'; + export interface TestExpression { text: string; - invalid: 'false' | 'true' | 'semantic'; + invalid: InvalidKind; } export interface TestLibrary { text: string; - invalid: 'false' | 'true' | 'semantic'; + invalid: InvalidKind; } export interface TestOutput { @@ -89,7 +91,7 @@ export interface InternalTestResult { testName: string; testVersion?: string; testVersionTo?: string; - invalid?: 'false' | 'true' | 'semantic' | 'undefined'; + invalid?: ResultInvalidKind; expression: string; // For library-style tests: the full CQL library source, sent to Library/$evaluate wrapped as // a FHIR Library resource. (For these tests, `expression` holds the name of the define whose @@ -113,7 +115,7 @@ export interface TestResult { testsName: string; groupName: string; testName: string; - invalid?: 'false' | 'true' | 'semantic'; + invalid?: InvalidKind; capabilities?: CapabilityKV[]; expression: string; } diff --git a/src/shared/invalid-utils.ts b/src/shared/invalid-utils.ts new file mode 100644 index 0000000..881fccf --- /dev/null +++ b/src/shared/invalid-utils.ts @@ -0,0 +1,40 @@ +/** + * Semantics of the test schema's `invalid` attribute. + * + * `testSchema.xsd` defines five values for `InvalidType`, four of which say the test expects the + * engine to fail — they differ only in *how* it is expected to fail: + * + * - `false` — the expression is expected to evaluate successfully + * - `syntax` — expected to produce a syntax error (the translator rejects it) + * - `semantic` — expected to produce a semantic error (the translator rejects it) + * - `execution` — expected to produce an execution error + * - `true` — expected to produce a runtime error + * + * Treating only `true` and `semantic` as error-expecting scored a test as failed when the engine + * had in fact produced the syntax error the test asked for. + */ +export type InvalidKind = 'false' | 'syntax' | 'semantic' | 'execution' | 'true'; + +/** `invalid` as recorded on a result, which also covers a test that declares no expression. */ +export type ResultInvalidKind = InvalidKind | 'undefined'; + +const ERROR_EXPECTED: ReadonlySet = new Set(['true', 'semantic', 'syntax', 'execution']); + +/** + * True when the test expects the engine to error, whatever kind of error that is. Such a test + * passes only if the engine actually errored, and needs no `` to compare against. + */ +export function expectsError(invalid: string | undefined): boolean { + return invalid !== undefined && ERROR_EXPECTED.has(invalid); +} + +const TRANSLATION_ERRORS: ReadonlySet = new Set(['semantic', 'syntax']); + +/** + * True when the expression cannot survive CQL-to-ELM translation, so it must be left out of a + * generated library — one such define would fail the translation of every define around it. + * Runtime and execution errors translate fine and are kept. + */ +export function preventsTranslation(invalid: string | undefined): boolean { + return invalid !== undefined && TRANSLATION_ERRORS.has(invalid); +} diff --git a/src/shared/results-shared.ts b/src/shared/results-shared.ts index f41e21d..56b4599 100644 --- a/src/shared/results-shared.ts +++ b/src/shared/results-shared.ts @@ -1,5 +1,7 @@ import type { Tests, Test, InternalTestResult, CapabilityKV } from '../models/test-types.js'; import type { Parameters } from 'fhir/r4'; +import { expectsError } from './invalid-utils.js'; +import type { ResultInvalidKind } from './invalid-utils.js'; /** * The XML parser yields a bare object when an element declares exactly one @@ -31,7 +33,7 @@ export class Result implements InternalTestResult { testName: string; testVersion?: string; testVersionTo?: string; - invalid: 'false' | 'true' | 'semantic' | 'undefined'; + invalid: ResultInvalidKind; expression: string; library?: string; capability: CapabilityKV[] = []; @@ -99,10 +101,10 @@ export class Result implements InternalTestResult { } else { this.expected = test.output as string; } - } 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. + } else if (!expectsError(this.invalid)) { + // No output is expected only when the expression is marked invalid — any of the four + // error kinds means the test expects the engine to fail. Otherwise there is nothing + // to compare against, so skip. this.testStatus = 'skip'; this.skipMessage = 'No output specified'; } diff --git a/src/shared/run-test-core.ts b/src/shared/run-test-core.ts index c49b4a7..0303ece 100644 --- a/src/shared/run-test-core.ts +++ b/src/shared/run-test-core.ts @@ -12,6 +12,7 @@ import { resultsEqual } from './results-utils.js'; import { formatActualValue } from '../test-results/cql-test-results.js'; import { publishTestLibrary } from './library-publisher.js'; import type { PublishedLibrary } from './library-publisher.js'; +import { expectsError } from './invalid-utils.js'; /** * Shared execution state for a test run: the resolved config, the engine, the CVL parser, @@ -121,8 +122,8 @@ function logSkip(result: InternalTestResult): void { /** * Runs a single test against the engine and records its outcome on `result`. Applies skip * precedence (pre-marked skip → OnlyList → config SkipList → version gating), then POSTs the - * expression, extracts the actual value, and classifies pass/fail/error. Errors expected by - * `invalid="true"/"semantic"` tests pass only when the engine actually erred. + * expression, extracts the actual value, and classifies pass/fail/error. A test whose `invalid` + * attribute names any of the four error kinds passes only when the engine actually erred. * * This is the single implementation shared by the CLI and server runners — both use `fetch` and * identical classification, so a test scores the same regardless of how it is invoked. @@ -206,8 +207,10 @@ export async function runTest( const invalid = result.invalid; const erroredOut = responseIndicatesError(response.status, responseBody); - if (invalid === 'true' || invalid === 'semantic') { - // The expression is expected to error; it passes only if the engine erred. + if (expectsError(invalid)) { + // The expression is expected to error; it passes only if the engine erred. All four + // error kinds (syntax, semantic, execution, true) are treated alike here — the test + // asks for a failure, and the runner does not police which kind the engine produced. result.testStatus = erroredOut ? 'pass' : 'fail'; } else if (!erroredOut) { result.testStatus = resultsEqual(parsedExpected, result.actual) ? 'pass' : 'fail'; diff --git a/src/test-results/cql-test-results.ts b/src/test-results/cql-test-results.ts index bb84124..01b9f59 100644 --- a/src/test-results/cql-test-results.ts +++ b/src/test-results/cql-test-results.ts @@ -5,6 +5,7 @@ import type { TestResult, InternalTestResult } from '../models/test-types.js'; import type { TestResultsSummary, CQLTestResultsData } from '../models/results-types.js'; import { ResultsValidator } from '../conf/results-validator.js'; import { isIntervalShaped } from '../shared/interval-utils.js'; +import type { InvalidKind } from '../shared/invalid-utils.js'; /** * Formats an actual value for report output. Structured CQL values are rendered in @@ -209,7 +210,7 @@ export class CQLTestResults { }), ...(result.invalid && result.invalid !== 'undefined' && { - invalid: result.invalid as 'false' | 'true' | 'semantic', + invalid: result.invalid as InvalidKind, }), ...(result.capability && result.capability.length > 0 && { capabilities: result.capability }), diff --git a/test/invalid-utils.test.ts b/test/invalid-utils.test.ts new file mode 100644 index 0000000..506a473 --- /dev/null +++ b/test/invalid-utils.test.ts @@ -0,0 +1,72 @@ +import { describe, it, expect } from 'vitest'; +import { expectsError, preventsTranslation } from '../src/shared/invalid-utils.js'; +import { Result } from '../src/shared/results-shared.js'; + +/** + * testSchema.xsd defines five InvalidType values. Four of them say the test expects the engine to + * fail; only `false` expects success. + */ +const ERROR_KINDS = ['true', 'semantic', 'syntax', 'execution'] as const; + +describe('expectsError', () => { + it('is true for every error kind the schema defines', () => { + for (const kind of ERROR_KINDS) { + expect(expectsError(kind), kind).toBe(true); + } + }); + + it('is false for a test expected to succeed, or with no invalid attribute', () => { + expect(expectsError('false')).toBe(false); + expect(expectsError(undefined)).toBe(false); + // The runner's own marker for a test that declares no expression at all. + expect(expectsError('undefined')).toBe(false); + }); + + it('is false for an unrecognised value rather than treating it as an error', () => { + expect(expectsError('nonsense')).toBe(false); + expect(expectsError('')).toBe(false); + }); +}); + +describe('preventsTranslation', () => { + it('is true only for errors the translator raises', () => { + // A define that cannot translate would fail the whole generated library. + expect(preventsTranslation('syntax')).toBe(true); + expect(preventsTranslation('semantic')).toBe(true); + }); + + it('is false for run-time and execution errors, which translate fine', () => { + expect(preventsTranslation('true')).toBe(false); + expect(preventsTranslation('execution')).toBe(false); + }); + + it('is false for a valid expression or a missing attribute', () => { + expect(preventsTranslation('false')).toBe(false); + expect(preventsTranslation(undefined)).toBe(false); + }); +}); + +describe('Result treats every error kind as needing no output', () => { + // A test that expects an error carries no ; before, only `true` and `semantic` + // were recognised, so a syntax-invalid test with no output was skipped as + // "No output specified" instead of being run. + for (const kind of ERROR_KINDS) { + it(`does not skip an invalid="${kind}" test that declares no output`, () => { + const result = new Result('T', 'G', { + name: 'ErrorExpected', + expression: { text: 'Ceiling(2147483648)', invalid: kind as any }, + } as any); + expect(result.testStatus).not.toBe('skip'); + expect(result.skipMessage).toBeUndefined(); + }); + } + + it('still skips a valid test that declares no output', () => { + const result = new Result('T', 'G', { + name: 'NoOutput', + expression: '1 + 1', + } as any); + expect(result.testStatus).toBe('skip'); + expect(result.skipMessage).toBe('No output specified'); + }); +});