Skip to content

added tests for timezone offset tests#88

Open
bryantaustin13 wants to merge 3 commits into
cqframework:mainfrom
bryantaustin13:issue81_Timezoneoffset-policy-tests
Open

added tests for timezone offset tests#88
bryantaustin13 wants to merge 3 commits into
cqframework:mainfrom
bryantaustin13:issue81_Timezoneoffset-policy-tests

Conversation

@bryantaustin13

Copy link
Copy Markdown
Contributor

uses codes from HL7/cql#103 (comment)

Answers this issue
#81 -- Timezoneoffset policy tests

@raleigh-g-thompson raleigh-g-thompson left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Incorrect outputs on the three "NullWhen..." tests in the "No default offset" group — shouldn't these be null rather than true?
  2. Missing version attribute — why is there no version attribute on the Policy: Default to server offset (normalize offset-less to server) group?
  3. {{SERVER_OFFSET_ISO}} placeholder — could you add a comment explaining the runtime substitution mechanism to help future maintainers?
  4. New capability codes — are timezone-offset-policy.no-default-offset and timezone-offset-policy.default-server-offset documented somewhere, or is that planned as a follow-up?

Comment thread tests/cql/CQLTimeZoneOffsetTest.xml
Comment thread tests/cql/CQLTimeZoneOffsetTest.xml Outdated
Comment thread tests/cql/CQLTimeZoneOffsetTest.xml
<capability code="component-extraction"/>
<capability code="timezone-offset-policy.no-default-offset"/><!-- Offset-less vs explicit UTC -->
<expression>@2020-01-01T12:00:00.000 = @2020-01-01T12:00:00.000+06:00</expression>
<output>null</output><!-- Bad test:: Depending on what “If no timezone offset is specified, the timezone offset of the evaluation request timestamp is used.” -->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an invalid test, the spec is clear that timezone offset must be defaulted to the timezone offset of the evaluation request.

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.

3 participants