Duration uncertainty

[elsaperelli]

Summary

This PR updates the functionality of durationBetween calculations in order to match the latest descriptions in the CQL Logical Specification described here. This PR also fixes #325.

According to the CQL Logical Specification, "Note that uncertainty calculations, just like date and time comparison calculations, consider seconds and milliseconds as a single combined precision with decimal semantics". This means that the duration between calculation should NOT produce an Uncertainty in both cases: when ms are defined and when ms are not defined.

Pull requests into cql-execution require the following.
Submitter and reviewer should ✔ when done.
For items that are not-applicable, mark "N/A" and ✔.

Submitter:

• This pull request describes why these changes were made
• Code diff has been done and been reviewed (it does not contain: additional white space, not applicable code changes, debug statements, etc.)
• Tests are included and test edge cases
• Tests have been run locally and pass
• Code coverage has not gone down and all code touched or added is covered.
• Code passes lint and prettier (hint: use npm run test:plus to run tests, lint, and prettier)
• All dependent libraries are appropriately updated or have a corresponding PR related to this change
• cql4browsers.js built with npm run build:browserify if source changed.

Reviewer:

Name:

• Code is maintainable and reusable, reuses existing code and infrastructure where appropriate, and accomplishes the task’s purpose
• The tests appropriately test the new code, including edge cases
• You have tried to break the code

Additional Testing Guidance

• Recommend testing the measure bundle and two test cases provided in Uncertainty Produced for Duration Operation Involving Days Granularity  #325 with fqm-execution with this branch of cql-execution linked

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.46%. Comparing base (89699b5) to head (50e3ac1).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #340      +/-   ##
==========================================
+ Coverage   87.43%   87.46%   +0.02%     
==========================================
  Files          52       52              
  Lines        4536     4546      +10     
  Branches     1280     1284       +4     
==========================================
+ Hits         3966     3976      +10     
  Misses        361      361              
  Partials      209      209              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cmoesel]

Thanks, @elsaperelli. I think the approach is right on, but I'm made a small suggestion for your consideration and I think I also discovered a tiny mistake in the tests.

[cmoesel]

Forgive me for not thinking of this before, but... I think that the approach only holds true for durations when the precision is seconds and above.

In the Date, DateTime, and Time section of the Author's guide, it says (emphasis mine):

In other words, when performing comparisons or calculations for precisions of seconds and above, if milliseconds are not specified, the calculation should be performed as though milliseconds had been specified as 0.

And in the DateTime and Time sections of the reference it says something similar (again, emphasis mine):

In other words, if milliseconds are not specified, calculations for precisions above milliseconds should be performed as though milliseconds had been specified as 0.

So if the expression is milliseconds between A and B then we should not convert null milliseconds to 0. I think this means that in the duration code, we additionally need to check the unitField argument of durationBetween to make sure it's not milliseconds. Do you agree with that interpretation of the spec?

[cmoesel]

Thank you, Elsa! This seems to do the trick!

[hossenlopp]

Looks good to me!