-
Notifications
You must be signed in to change notification settings - Fork 324
Tests for Compiling CQL with Roboelectric #1395
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1395 +/- ##
============================================
+ Coverage 76.53% 76.68% +0.15%
- Complexity 815 841 +26
============================================
Files 141 144 +3
Lines 4663 4779 +116
Branches 850 863 +13
============================================
+ Hits 3569 3665 +96
- Misses 623 635 +12
- Partials 471 479 +8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
ktarasenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great one!
workflow/src/test/java/com/google/android/fhir/workflow/CQLBuilderUtils.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLBuilderUtils.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
|
@jingtang10 @ktarasenko I added a bunch of little improvements on documentation and code readability. I hope it helps. |
|
is there anything blocking this from getting merged? we're going to add some test cases on top if this pr |
|
hum... let me fix these conflicts. |
# Conflicts: # workflow/build.gradle.kts
jingtang10
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Vitor!
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CQLCompilerTest.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CqlBuilderUtils.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CqlBuilderUtils.kt
Outdated
Show resolved
Hide resolved
workflow/src/test/java/com/google/android/fhir/workflow/CqlBuilderUtils.kt
Show resolved
Hide resolved
jingtang10
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should do more work on documentation. How can this be used by developers? What advantages/disadvantages? Let's consider improving the github wiki regarding this?
For now this PR itself is fine.
thanks vitor
|
Thanks Jing! I agree with the need to turn some of this into documentation, which frankly was what triggered this PR in the first place: people not knowing how to use the evaluator to compile and build CQL-based FHIR Libraries manually. Generating FHIRLibraries should be solved by appropriate tooling, but meanwhile, our documentation can fill the gap. I also see good use of the testing platform to make sure the documentation is accurate and up-to-date. |
|
@vitorpamplona , since the PR has been approved, when do we intend to merge these changes? |
@vitorpamplona , since the PR is approved, when can the changes get merged? |
|
Superseded by #1603 |
IMPORTANT: All PRs must be linked to an issue (except for extremely trivial and straightforward changes).
Starts to fix #1365
Description
Adds short tests to compile CQL into ELM, assemble ELM into a FHIR Library and making a FHIR Bundle from it.
Alternative(s) considered
No
Type
Testing
Screenshots (if applicable)
Checklist
./gradlew spotlessApplyand./gradlew spotlessCheckto check my code follows the style guide of this project../gradlew checkand./gradlew connectedCheckto test my changes locally.