-
Notifications
You must be signed in to change notification settings - Fork 165
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
Update Azure Pipeline to Test Recent JDKs #622
Conversation
Codecov Report
@@ Coverage Diff @@
## master #622 +/- ##
==========================================
- Coverage 9.56% 9.56% -0.01%
==========================================
Files 1706 1706
Lines 534901 534901
Branches 159144 159144
==========================================
- Hits 51150 51148 -2
Misses 464179 464179
- Partials 19572 19574 +2
Continue to review full report at Codecov.
|
pull-request-pipeline.yml
Outdated
parameters: | ||
imageName: [ 'ubuntu-latest', 'macos-latest', 'windows-2019' ] | ||
jdkVersion: [ '1.8', '1.11'] |
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.
Should we add JDK 17 support? It's the most recent LTS version of Java.
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.
add it in, yes
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.
JDK 17 appears to work with macos and windows, but ubuntu reported Failed to find the specified JDK version.
imageName: "macos-latest" | ||
windows: | ||
imageName: "windows-2019" | ||
maxParallel: 3 |
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.
Do we want to add this back in? If so should it be set to 6?
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.
these appear to be covered in the imageName bracketed arguments above? is that not the case?
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.
Sorry I meant maxParallel
specifically. The imageNames are covered.
Without maxParallel more than 3 jobs will run simultaneously. As I type 7 of the 9 jobs are running (2 have already failed)
@markiantorno any other changes you'd like to see in this PR? |
@dotasek thanks for making those fixes/updates? It looks like there isn't a more recent PR where I can see the checks in GitHub, but I can see them happening in Azure Pipelines for your PR. I think the matrix approach I took was overkill now that I see what your config looks like 😅. Once a new release is out I'll test to see if any of the fixes you implemented address the issue that originally brought this to light. details here and in the zulip thread linked, but TL;DR is that the profiles fetched when loading an IG with the validator varied by JDK version. |
Making updates to the Azure Pipeline so that it tests PRs against multiple JDK Versions. I haven't used Azure Pipelines before, so there might be a better way to do this.
Currently, it doesn't appear that Azure Pipelines support a cross-product matrix strategy like GitHub Actions, but there are some examples of how to replicate the functionality in Azure Pipelines.
fhir-validator-wrapper/41 highlighted a suspected issue that only appeared when using JDK 8 (relevant zulip thread).