Test Windows in CI #6824
Test Windows in CI #6824
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6824 +/- ##
============================================
- Coverage 90.49% 90.48% -0.02%
Complexity 6598 6598
============================================
Files 731 731
Lines 19735 19736 +1
Branches 1938 1938
============================================
- Hits 17860 17858 -2
- Misses 1283 1285 +2
- Partials 592 593 +1 ☔ View full report in Codecov by Sentry. |
| -Porg.gradle.java.installations.paths=${{ steps.setup-java-test.outputs.path }},${{ steps.setup-java.outputs.path }} | ||
| "-Porg.gradle.java.installations.paths=${{ steps.setup-java-test.outputs.path }}" | ||
| "-Porg.gradle.java.installations.auto-download=false" |
There was a problem hiding this comment.
I think the double quotes ended up being the only thing that was needed here, but the other changes seem good and align with the opentelemetry-java-instrumentation project (where, btw, we only run smoke tests against windows but not regular integration or unit tests, so haven't run into this double quote issue)
context/src/test/java/io/opentelemetry/context/internal/shaded/WeakConcurrentMapTest.java
Outdated
Show resolved
Hide resolved
| env: | ||
| GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }} | ||
| # JMH-based tests run only if this environment variable is set to true | ||
| RUN_JMH_BASED_TESTS: ${{ matrix.jmh-based-tests }} | ||
|
|
||
| - name: Check for diff | ||
| # The jApiCmp diff compares current to latest, which isn't appropriate for release branches, or for bot-generated PRs | ||
| if: ${{ !startsWith(github.ref_name, 'release/') && !startsWith(github.base_ref, 'release/') && (github.actor != 'opentelemetrybot') }} | ||
| if: ${{ matrix.os != 'windows-latest' && !startsWith(github.ref_name, 'release/') && !startsWith(github.base_ref, 'release/') && (github.actor != 'opentelemetrybot') }} |
There was a problem hiding this comment.
#nit: maybe update the comment to specify why its not required on windows?
| throw new ConfigurationException("Configuration file not found", e); | ||
| } | ||
| try { | ||
| try (FileInputStream fis = new FileInputStream(configurationFile)) { |
There was a problem hiding this comment.
Nice catch here to close this. Is is really the case that IOException is only thrown when an error occurs closing the file?
There was a problem hiding this comment.
yeah, I was kinda surprised by that too, I'll add a comment in that catch block
No description provided.