Repository navigation
Normalize a single <capability> element so it is not dropped - #121
Merged
Merged
Conversation
The XML parser yields a bare object when a test declares exactly one <capability> and an array only for two or more, but Result matched on Array.isArray alone and fell through to []. Every single-capability test therefore reported no capabilities: 7,390 tests in the suite, against 513 with two or more. Worst affected are CqlIntervalOperatorsTest (2,091), CqlListOperatorsTest (1,020) and CqlDateTimeOperatorsTest (1,006). Normalize to an array before mapping, and widen Test.capability to CapabilityKV | CapabilityKV[] so the declared type matches what the parser actually produces rather than hiding the case that was being missed. Impact is confined to reporting — capability feeds the capabilities field in the results file and the MCP payload, and does not gate execution — so no pass/fail verdict changes. Covered by tests that fail without the fix.
Closed
raleigh-g-thompson
approved these changes
Aug 18, 2026
raleigh-g-thompson
left a comment
Contributor
There was a problem hiding this comment.
No comment. Looks good.
bryantaustin13
added a commit
that referenced
this pull request
Aug 18, 2026
#117 deduped the CLI and server runners into shared/run-test-core.ts, and #121 widened Test.capability. Three conflicts, resolved as follows. test-runner.ts and test-execution-service.ts: took main's versions wholesale. This branch had added the same library logic twice, once per runner, and those copies no longer exist — the logic is ported into run-test-core.ts instead, so it now lives in one place and applies identically to both paths: - skip precedence gains a library-vs-$cql arm, since a whole CQL library can only be evaluated via Library/$evaluate - the library is published before the request and its canonical url passed to generateParametersResource - removal moved into a finally, so a failing test no longer leaves the resource behind The server copy also had result.SkipMessage with a capital S, which never reached the results file; the single ported version uses skipMessage. test-types.ts: combined both sides — this branch's optional expression plus library, and main's widened capability type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resultmatchedtest.capabilityonArray.isArrayalone and fell through to[]. The XML parser yields a bare object when a test declares exactly one<capability>element and an array only for two or more, so every single-capability test reported no capabilities at all — 7,390 tests in the suite, against 513 with two or more. Worst affected areCqlIntervalOperatorsTest(2,091),CqlListOperatorsTest(1,020) andCqlDateTimeOperatorsTest(1,006).Changes
results-shared.ts— normalizetest.capabilityto an array before mapping.test-types.ts— widenTest.capabilitytoCapabilityKV | CapabilityKV[], so the declared type matches what the parser actually produces instead of hiding the case that was being missed. Verified this does not cascade to other call sites.test/results-shared.test.ts— new; four tests covering one capability, several, none, and that normalization leaves the rest of theResultuntouched. Each fails without the fix.Impact is confined to reporting:
capabilityfeeds thecapabilitiesfield in the results file and the MCP server payload, and does not gate execution, so no pass/fail verdict changes.tsc --noEmitclean, 188 tests passing. Merges cleanly againstmain, and independently of #113 and #115 — it touches onlyresults-shared.tsandtest-types.ts, which neither of those PRs modifies.Replaces #114
#114 also carried invalid-test handling, all of which #116 has since delivered in a better form —
responseIndicatesErrorchecks the response status and looks structurally for anevaluation errorparameter, where #114 regex-matched the stringified actual value. TheparsedExpectedguard and the "invalid tests need no<output>" rule are also already onmain, and thesingletonListKeysguard is unnecessary becausesingletonListKeysFromExpectedalready returns an empty set for a non-array input.Only the capability fix remained, so this PR is that alone. #114 was raised from a branch name that no longer describes the change, and a PR's head branch cannot be repointed, hence the new PR.