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

FI-2727 Initial Implementation of Inferno (g)(10) Test Kit into CI #182

Merged
merged 57 commits into from
Nov 13, 2024

Conversation

Shaumik-Ashraf
Copy link
Contributor

@Shaumik-Ashraf Shaumik-Ashraf commented Oct 25, 2024

Summary

This PR is the culmination of work based off of various other branches and PRs (see #167, #180, inferno-framework/inferno-core#540, inferno-framework/inferno-core#542, inferno-framework/inferno-core#550, inferno-framework/tls-test-kit#20, onc-healthit/onc-certification-g10-test-kit#577). It utilizes the new inferno execute CLI to run Inferno tests in CI, and developers may look at the .github/workflows/inferno_ci.yml file as reference to implement their own CI pipelines.

Please note the current implementation has a few limitations, and certain tests such as TLS tests and SMART launch cannot be emulated in a typical CI environment. This PR's implementation currently disables TLS tests with the INFERNO_DISABLE_TLS_TEST environment variable. We are looking for better methods to properly integration test the remaining (g)(10) test kit and other Inferno test kit features.

New behavior

The following components of the ONC Certification (g)(10) Test Kit will now run as a GitHub Action Continuous Integration test on the Inferno Reference Server:

  • Single Patient API with US Core 3.1.1
  • Multi-Patient API with US Core 3.1.1 and Bulk Data 1.0.1
  • Single Patient API with US Core 6.1.0
  • Multi-Patient API with US Core 6.1.0 and Bulk Data 2.0.0

Code changes

Replace the wait-for-it script with wait-for, clean up old GitHub Action files, and implement the new .github/workflows/inferno_ci.yml file.

Testing guidance

Observe Github Actions result for Inferno CI workflow.

@Shaumik-Ashraf Shaumik-Ashraf changed the title Fi 2727 inferno ci disable tls FI-2727 Initial Implementation of Inferno (g)(10) Test Kit into CI Oct 28, 2024
@Shaumik-Ashraf Shaumik-Ashraf self-assigned this Oct 28, 2024
bulk_patient_ids_in_group:85,355 \
"smart_credentials:{\"access_token\":\"SAMPLE_TOKEN\"}"

- name: Run g10 test kit Single Patient API with US Core 6.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

If the 3.1.1 tests fail then this won't run and it could possibly hide a failure here as well. How would you feel about using a matrix with the different US core versions? https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/running-variations-of-jobs-in-a-workflow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but doing this as a matrix is another ticket (FI-3373)

Copy link
Contributor

@arscan arscan Oct 30, 2024

Choose a reason for hiding this comment

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

If you want to push this update to later, please update that other ticket so that it isn't just about doing more combos but it is ALSO about not halting on the first version that fails, which I agree would be annoying from a development planning perspective. I didn't realize that is how this works.

.github/workflows/inferno_ci.yml Show resolved Hide resolved
.github/workflows/inferno_ci.yml Outdated Show resolved Hide resolved
@Shaumik-Ashraf
Copy link
Contributor Author

Ready for re-review and merge!

@arscan
Copy link
Contributor

arscan commented Nov 13, 2024

Ready for re-review and merge!

Could you just quickly reply with what you did (if anything) in the above comment threads @Shaumik-Ashraf to expedite review?

.github/workflows/inferno_ci.yml Outdated Show resolved Hide resolved
.github/workflows/inferno_ci.yml Outdated Show resolved Hide resolved
Shaumik-Ashraf and others added 2 commits November 13, 2024 11:55
Co-authored-by: Dylan Hall <[email protected]>
@Shaumik-Ashraf
Copy link
Contributor Author

Ready for re-review and merge!

Could you just quickly reply with what you did (if anything) in the above comment threads @Shaumik-Ashraf to expedite review?

I'll summarize here:

  • I showcase inferno execute running multiple groups together by putting the Single Patient API and Multi-Patient API steps together
  • I have the CI always use the latest release of (g)(10), but I also left commented out examples of using a Test Kit's fixed release and main branch.
  • Just now I fixed and cleaned up my remaining comments. I'm trying to keep this workflow file dev friendly. Thanks @dehall for the catch

@Shaumik-Ashraf Shaumik-Ashraf merged commit 83d7c7d into main Nov 13, 2024
3 checks passed
@Shaumik-Ashraf Shaumik-Ashraf deleted the fi-2727-inferno-ci-disable-tls branch November 15, 2024 17:19
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