Skip to content

Drop cql-execution skips for tests that now pass, and relabel DateTimeToString2 - #149

Open
bryantaustin13 wants to merge 1 commit into
mainfrom
js-config-skip-cleanup
Open

bryantaustin13 wants to merge 1 commit into
mainfrom
js-config-skip-cleanup

Conversation

@bryantaustin13

Copy link
Copy Markdown
Contributor

Problem

conf/cql-execution-local.json skips some tests whose cause is already fixed on main, and one entry points at a test that no longer exists. I ran main against the cql-execution test-server (cql-execution 4.0.0-beta.1, cql-tests f591d9e, the commit main pins) with every skip removed, and checked each entry.

Skipped test Skip reason On main with the skip removed
ExpandPerHour, ExpandPerHourOpen #94 (lists of Interval<Time>) pass
ExpandPerHourIntervalOverload, ExpandPerHourOpenIntervalOverload #135 (time-precision; merged) pass
TestInNullBoundaries cqframework/cql-tests#69 does not exist: cql-tests#116 replaced it with TestIntegerInMaxIntervalTrue and TestIntegerInUnknownIntervalNull, which both pass

DateTimeToString2 is skipped as #84 (DateTime with vs without an offset), but #84 isn't the cause. With #145 applied, the test still fails: the result is the string '2000-01-01T15:25:25.300+00:00' and the test expects no offset. That's cqframework/cql-tests#73. Left as it is, removing the #84 skips after #145 merges would turn this one into a failure.

Change

conf/cql-execution-local.json only:

  • Removed the 5 entries above.
  • Relabelled DateTimeToString2 to Invalid Test: Wrong Expected Outcome for ToString of DateTime with No Offset Specified: https://github.com/cqframework/cql-tests/issues/73. It stays skipped.

The other skips are left alone:

Each of those can go once its PR merges.

Verification

Same server and tests:

Config Pass Fail Skip Error
main 1692 64 86 0
this PR 1696 64 82 0

The only status changes are the 4 tests above, skip → pass. DateTimeToString2 is still skipped, with the new reason. The diff also applies cleanly on top of #147.

cql-tests#69 can probably be closed, since the test it describes was replaced.

🤖 Generated with Claude Code

…eToString2

Run against the cql-execution test-server with every skip removed, four
skipped tests already pass on main: ExpandPerHour and ExpandPerHourOpen
(skipped for #94) and the two ExpandPerHour IntervalOverload tests
(skipped for #135, which has merged). The TestInNullBoundaries entry
points at a test cql-tests #116 replaced with TestIntegerInMaxIntervalTrue
and TestIntegerInUnknownIntervalNull, which both pass. All five entries
are removed.

DateTimeToString2 was skipped as #84, but it still fails with #145: the
test expects a DateTime string without an offset, which is cql-tests #73.
Its reason now says so, so it stays skipped when the #84 skips go.

On cql-execution: 1692 -> 1696 pass, 86 -> 82 skip, failures unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant