From ff4fe978e55d6afa0e8a9d7c3e2674c4083332f7 Mon Sep 17 00:00:00 2001 From: Trask Stalnaker Date: Fri, 1 Nov 2024 07:01:14 -0700 Subject: [PATCH] Test Windows in CI (#6824) --- .github/workflows/build.yml | 8 ++++++-- .../internal/shaded/WeakConcurrentMapTest.java | 12 ++++++++---- .../exporter/otlp/internal/OtlpConfigUtil.java | 3 +-- .../PrometheusMetricReaderProviderTest.java | 3 ++- .../okhttp/internal/RetryInterceptorTest.java | 2 +- .../opentracingshim/SpanShimTest.java | 2 +- .../AutoConfiguredOpenTelemetrySdkBuilder.java | 14 +++++++------- .../fileconfig/FileConfigurationCreateTest.java | 10 +++++++--- .../testing/assertj/AttributeAssertionTest.java | 5 ++++- .../sdk/testing/assertj/TraceAssertionsTest.java | 3 ++- 10 files changed, 39 insertions(+), 23 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 1dfc5fb6c7..a7a4430a67 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -23,6 +23,7 @@ jobs: - macos-latest - macos-13 - ubuntu-latest + - windows-latest test-java-version: - 8 - 11 @@ -71,7 +72,8 @@ jobs: ./gradlew build ${{ matrix.coverage && 'jacocoTestReport' || '' }} -PtestJavaVersion=${{ matrix.test-java-version }} - -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" env: GRADLE_ENTERPRISE_ACCESS_KEY: ${{ secrets.GRADLE_ENTERPRISE_ACCESS_KEY }} # JMH-based tests run only if this environment variable is set to true @@ -79,7 +81,9 @@ jobs: - 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') }} + # this fails on windows because of the bash-specific if/then/else syntax, but that's ok + # because we only need to run this validation once (on any platform) + if: ${{ matrix.os != 'windows-latest' && !startsWith(github.ref_name, 'release/') && !startsWith(github.base_ref, 'release/') && (github.actor != 'opentelemetrybot') }} run: | # need to "git add" in case any generated files did not already exist git add docs/apidiffs diff --git a/context/src/test/java/io/opentelemetry/context/internal/shaded/WeakConcurrentMapTest.java b/context/src/test/java/io/opentelemetry/context/internal/shaded/WeakConcurrentMapTest.java index 3d635be779..6adfb18821 100644 --- a/context/src/test/java/io/opentelemetry/context/internal/shaded/WeakConcurrentMapTest.java +++ b/context/src/test/java/io/opentelemetry/context/internal/shaded/WeakConcurrentMapTest.java @@ -25,6 +25,7 @@ package io.opentelemetry.context.internal.shaded; +import static org.awaitility.Awaitility.await; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; @@ -75,8 +76,7 @@ class WeakConcurrentMapTest { assertThat(map.getCleanerThread(), not(nullValue(Thread.class))); new MapTestCase(map).doTest(); map.getCleanerThread().interrupt(); - Thread.sleep(200L); - assertThat(map.getCleanerThread().isAlive(), is(false)); + await().untilAsserted(() -> assertThat(map.getCleanerThread().isAlive(), is(false))); } static class KeyEqualToWeakRefOfItself { @@ -152,8 +152,12 @@ class WeakConcurrentMapTest { assertThat(values.isEmpty(), is(true)); key1 = key2 = null; // Make eligible for GC System.gc(); - Thread.sleep(200L); - triggerClean(); + await() + .untilAsserted( + () -> { + triggerClean(); + assertThat(map.approximateSize(), is(2)); + }); assertThat(map.get(key3), is(value3)); assertThat(map.getIfPresent(key3), is(value3)); assertThat(map.get(key4), is(value4)); diff --git a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java index 5040280850..999fc412a5 100644 --- a/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java +++ b/exporters/otlp/all/src/main/java/io/opentelemetry/exporter/otlp/internal/OtlpConfigUtil.java @@ -322,8 +322,7 @@ public final class OtlpConfigUtil { if (!file.exists()) { throw new ConfigurationException("Invalid OTLP certificate/key path: " + filePath); } - try { - RandomAccessFile raf = new RandomAccessFile(file, "r"); + try (RandomAccessFile raf = new RandomAccessFile(file, "r")) { byte[] bytes = new byte[(int) raf.length()]; raf.readFully(bytes); return bytes; diff --git a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java index 7945514af9..1c00a604e1 100644 --- a/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java +++ b/exporters/prometheus/src/test/java/io/opentelemetry/exporter/prometheus/internal/PrometheusMetricReaderProviderTest.java @@ -96,7 +96,8 @@ class PrometheusMetricReaderProviderTest { .extracting("server", as(InstanceOfAssertFactories.type(HttpServer.class))) .satisfies( server -> { - assertThat(server.getAddress().getHostName()).isEqualTo("localhost"); + assertThat(server.getAddress().getHostName()) + .isIn("localhost", "127.0.0.1", "kubernetes.docker.internal"); assertThat(server.getAddress().getPort()).isEqualTo(port); }); assertThat(metricReader.getMemoryMode()).isEqualTo(MemoryMode.IMMUTABLE_DATA); diff --git a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptorTest.java b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptorTest.java index 1a6656c621..8b47700e88 100644 --- a/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptorTest.java +++ b/exporters/sender/okhttp/src/test/java/io/opentelemetry/exporter/sender/okhttp/internal/RetryInterceptorTest.java @@ -172,7 +172,7 @@ class RetryInterceptorTest { client .newCall(new Request.Builder().url("http://localhost:" + openPort).build()) .execute()) - .isInstanceOf(ConnectException.class); + .isInstanceOfAny(ConnectException.class, SocketTimeoutException.class); verify(isRetryableException, times(5)).apply(any()); // Should retry maxAttempts, and sleep maxAttempts - 1 times diff --git a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanShimTest.java b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanShimTest.java index 4014739e2f..f681efe393 100644 --- a/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanShimTest.java +++ b/opentracing-shim/src/test/java/io/opentelemetry/opentracingshim/SpanShimTest.java @@ -135,7 +135,7 @@ class SpanShimTest { IntStream.range(0, baggageItemsCount) .forEach(i -> executor.execute(() -> spanShim.setBaggageItem("key-" + i, "value-" + i))); executor.shutdown(); - executor.awaitTermination(5, TimeUnit.SECONDS); + executor.awaitTermination(10, TimeUnit.SECONDS); for (int i = 0; i < baggageItemsCount; i++) { assertThat(spanShim.getBaggageItem("key-" + i)).isEqualTo("value-" + i); diff --git a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java index 995417b8fd..1c293a002c 100644 --- a/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java +++ b/sdk-extensions/autoconfigure/src/main/java/io/opentelemetry/sdk/autoconfigure/AutoConfiguredOpenTelemetrySdkBuilder.java @@ -541,13 +541,7 @@ public final class AutoConfiguredOpenTelemetrySdkBuilder implements AutoConfigur return null; } logger.fine("Autoconfiguring from configuration file: " + configurationFile); - FileInputStream fis; - try { - fis = new FileInputStream(configurationFile); - } catch (FileNotFoundException e) { - throw new ConfigurationException("Configuration file not found", e); - } - try { + try (FileInputStream fis = new FileInputStream(configurationFile)) { Class configurationFactory = Class.forName("io.opentelemetry.sdk.extension.incubator.fileconfig.FileConfiguration"); Method parse = configurationFactory.getMethod("parse", InputStream.class); @@ -567,6 +561,8 @@ public final class AutoConfiguredOpenTelemetrySdkBuilder implements AutoConfigur // resource return AutoConfiguredOpenTelemetrySdk.create( sdk, Resource.getDefault(), null, structuredConfigProperties); + } catch (FileNotFoundException e) { + throw new ConfigurationException("Configuration file not found", e); } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException e) { throw new ConfigurationException( "Error configuring from file. Is opentelemetry-sdk-extension-incubator on the classpath?", @@ -577,6 +573,10 @@ public final class AutoConfiguredOpenTelemetrySdkBuilder implements AutoConfigur throw (ConfigurationException) cause; } throw new ConfigurationException("Unexpected error configuring from file", e); + } catch (IOException e) { + // IOException (other than FileNotFoundException which is caught above) is only thrown + // above by FileInputStream.close() + throw new ConfigurationException("Error closing file", e); } } diff --git a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigurationCreateTest.java b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigurationCreateTest.java index 4ed182f03f..1230ce7383 100644 --- a/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigurationCreateTest.java +++ b/sdk-extensions/incubator/src/test/java/io/opentelemetry/sdk/extension/incubator/fileconfig/FileConfigurationCreateTest.java @@ -81,12 +81,16 @@ class FileConfigurationCreateTest { String rewrittenExampleContent = exampleContent .replaceAll( - "certificate: .*\n", "certificate: " + certificatePath + System.lineSeparator()) + "certificate: .*\n", + "certificate: " + certificatePath.replace("\\", "\\\\") + System.lineSeparator()) .replaceAll( - "client_key: .*\n", "client_key: " + clientKeyPath + System.lineSeparator()) + "client_key: .*\n", + "client_key: " + clientKeyPath.replace("\\", "\\\\") + System.lineSeparator()) .replaceAll( "client_certificate: .*\n", - "client_certificate: " + clientCertificatePath + System.lineSeparator()); + "client_certificate: " + + clientCertificatePath.replace("\\", "\\\\") + + System.lineSeparator()); InputStream is = new ByteArrayInputStream(rewrittenExampleContent.getBytes(StandardCharsets.UTF_8)); diff --git a/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/AttributeAssertionTest.java b/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/AttributeAssertionTest.java index aa229eefb3..44c09e39af 100644 --- a/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/AttributeAssertionTest.java +++ b/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/AttributeAssertionTest.java @@ -23,6 +23,9 @@ class AttributeAssertionTest { .getAssertion() .accept(AttributeAssertion.attributeValueAssertion(key, null))) .isInstanceOf(AssertionError.class) - .hasMessage("[STRING attribute 'flib'] \nExpecting actual not to be null"); + .hasMessage( + "[STRING attribute 'flib'] " + + System.lineSeparator() + + "Expecting actual not to be null"); } } diff --git a/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceAssertionsTest.java b/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceAssertionsTest.java index 18ac384969..feb5eb9462 100644 --- a/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceAssertionsTest.java +++ b/sdk/testing/src/test/java/io/opentelemetry/sdk/testing/assertj/TraceAssertionsTest.java @@ -687,7 +687,8 @@ class TraceAssertionsTest { .hasTracesSatisfyingExactly( trace -> trace.hasSpansSatisfyingExactly(span -> span.hasSpanId(SPAN_ID1)))) .isInstanceOf(AssertionError.class) - .hasMessageStartingWith("[Trace 0] \n" + "Expected size: 1 but was: 2"); + .hasMessageStartingWith( + "[Trace 0] " + System.lineSeparator() + "Expected size: 1 but was: 2"); // test asserting spans in wrong oder assertThatThrownBy(