Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial must supports added to data requirement #294

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

Conversation

lmd59
Copy link
Contributor

@lmd59 lmd59 commented Feb 26, 2024

Summary

This is an in progress PR to improve our data requirements and add mustSupport properties.

Current state

Currently all mustSupports match elm-parser-for-ecqms mustSupports for CMS2 (as long as multiple data requirements with the same vs/code are lined up properly) and CMS130 for first level properties (i.e. comparing "diagnosis", but not "diagnosis.condition"). The logic finds all properties, then goes through each retrieve to see if the property is applicable to that retrieve. The logic does a lot of ELM traversal and so is slow at this point and would benefit from further optimization

Next steps

We're currently paused on this effort because we may need a new approach to capture all must Supports generically. Looking at elm-parser-for-ecqms may be helpful, but because it uses the xPath, there may be some approaches taken there that are unavailable to us. Our current plan is to next create a function that takes an expression and figures out what retrieves are resultant from that expression. This will be difficult to create but would let us simplify the complexity of matching properties with retrieves and allow for a more general solution.

May not be useful, but another potential help is that we may be able to keep track of more as we create the initial retrieves structure and track what a retrieve is called on the way (i.e. alias, expression references, operands) and the associated context of that name?

New behavior

Code changes

Testing guidance

Copy link

github-actions bot commented Feb 26, 2024

Coverage report

Caution

Test run failed

St.
Category Percentage Covered / Total
🟡 Statements
77.72% (-7.43% 🔻)
2337/3007
🟡 Branches
66.31% (-6.7% 🔻)
2153/3247
🟡 Functions
78.9% (-8.58% 🔻)
415/526
🟡 Lines
78.4% (-7.07% 🔻)
2257/2879
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / propertyPaths.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / ValueSetHelper.ts
88.57% (-1.43% 🔻)
83.58%
80% (-5% 🔻)
88.41% (-1.45% 🔻)
🟢
... / QueryFilterParser.ts
86.69%
80.77% (-0.16% 🔻)
97.62% 86.47%
🟡
... / ELMHelpers.ts
73.33% (-26.67% 🔻)
55% (-45% 🔻)
50% (-50% 🔻)
74.07% (-25.93% 🔻)
🔴
... / DataRequirementHelpers.ts
14.57% (-51.69% 🔻)
9.38% (-55.44% 🔻)
6.56% (-51.78% 🔻)
15.48% (-51.61% 🔻)

Test suite run failed

Failed tests: 0/438. Failed suites: 1/31.
  ● Test suite failed to run

    test/unit/DataRequirementHelpers.test.ts:544:37 - error TS2339: Property 'findSourcewithScope' does not exist on type 'typeof import("/home/runner/work/fqm-execution/fqm-execution/src/helpers/DataRequirementHelpers")'.

    544       expect(DataRequirementHelpers.findSourcewithScope(expression, 'testScope')).toEqual(source);
                                            ~~~~~~~~~~~~~~~~~~~

Report generated by 🧪jest coverage report action from 5473102

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.

None yet

1 participant