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.

Sign up for GitHub

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

Stop publishing test fixtures with opentelemetry-api #6695

Merged
merged 2 commits into from
Sep 10, 2024
Merged

Stop publishing test fixtures with opentelemetry-api #6695

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7c3f591
Stop publishing test fixtures with opentelemetry-api
jack-berg Sep 9, 2024
96890a6
PR feedback
jack-berg Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions api/all/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
plugins {
id("otel.java-conventions")
id("otel.publish-conventions")
id("java-test-fixtures")
Copy link
Member Author

Choose a reason for hiding this comment

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

In hindsight, adding java-test-fixtures was overly hasty, since as shown in #6693 it impacts the pom.xml and publishes additional artifacts we didn't intend / account for. When we need to share test code in this repository, we simply add a *:testing-internal module and omit the otel.publish-conventions. I suspect this pattern was selected in the past after investigating test fixtures and seeing these types of side affects.

Screenshot 2024-09-09 at 10 07 28 AM

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI @zeitlinger

Copy link
Member

Choose a reason for hiding this comment

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

could we have detected this before merging?


id("otel.jmh-conventions")
id("otel.animalsniffer-conventions")
Expand All @@ -16,12 +15,10 @@ dependencies {

annotationProcessor("com.google.auto.value:auto-value")

testImplementation(project(":api:testing-internal"))

testImplementation("edu.berkeley.cs.jqf:jqf-fuzz")
testImplementation("com.google.guava:guava-testlib")
testFixturesApi(project(":testing-internal"))
testFixturesApi("junit:junit")
testFixturesApi("org.assertj:assertj-core")
testFixturesApi("org.mockito:mockito-core")
}

tasks.test {
Expand Down
1 change: 1 addition & 0 deletions api/all/src/test/java/io/opentelemetry/api/OpenTelemetryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import io.opentelemetry.api.logs.LoggerProvider;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.testing.internal.AbstractOpenTelemetryTest;
import io.opentelemetry.api.trace.TracerProvider;

class OpenTelemetryTest extends AbstractOpenTelemetryTest {
Expand Down
2 changes: 2 additions & 0 deletions api/all/src/test/java/io/opentelemetry/api/logs/DefaultLoggerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.api.logs;

import io.opentelemetry.api.testing.internal.AbstractDefaultLoggerTest;

class DefaultLoggerTest extends AbstractDefaultLoggerTest {

@Override
Expand Down
2 changes: 2 additions & 0 deletions api/all/src/test/java/io/opentelemetry/api/metrics/DefaultMeterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.api.metrics;

import io.opentelemetry.api.testing.internal.AbstractDefaultMeterTest;

public class DefaultMeterTest extends AbstractDefaultMeterTest {

@Override
Expand Down
2 changes: 2 additions & 0 deletions api/all/src/test/java/io/opentelemetry/api/trace/DefaultTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

package io.opentelemetry.api.trace;

import io.opentelemetry.api.testing.internal.AbstractDefaultTracerTest;

class DefaultTracerTest extends AbstractDefaultTracerTest {

@Override
Expand Down
2 changes: 1 addition & 1 deletion api/incubator/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ dependencies {
annotationProcessor("com.google.auto.value:auto-value")

testImplementation(project(":sdk:testing"))
testImplementation(testFixtures(project(":api:all")))
testImplementation(project(":api:testing-internal"))

testImplementation("io.opentelemetry.semconv:opentelemetry-semconv-incubating")

Expand Down
2 changes: 1 addition & 1 deletion api/incubator/src/test/java/io/opentelemetry/api/incubator/ExtendedOpenTelemetryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.AbstractOpenTelemetryTest;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.incubator.logs.ExtendedDefaultLoggerProvider;
import io.opentelemetry.api.incubator.logs.ExtendedLogger;
Expand All @@ -17,6 +16,7 @@
import io.opentelemetry.api.incubator.trace.ExtendedTracer;
import io.opentelemetry.api.logs.LoggerProvider;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.testing.internal.AbstractOpenTelemetryTest;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import org.junit.jupiter.api.Test;
Expand Down
2 changes: 1 addition & 1 deletion ...ncubator/src/test/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLoggerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.common.Value;
import io.opentelemetry.api.logs.AbstractDefaultLoggerTest;
import io.opentelemetry.api.logs.Logger;
import io.opentelemetry.api.logs.LoggerProvider;
import io.opentelemetry.api.testing.internal.AbstractDefaultLoggerTest;
import org.junit.jupiter.api.Test;

class ExtendedDefaultLoggerTest extends AbstractDefaultLoggerTest {
Expand Down
2 changes: 1 addition & 1 deletion ...ubator/src/test/java/io/opentelemetry/api/incubator/metrics/ExtendedDefaultMeterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.metrics.AbstractDefaultMeterTest;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.testing.internal.AbstractDefaultMeterTest;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

Expand Down
2 changes: 1 addition & 1 deletion ...cubator/src/test/java/io/opentelemetry/api/incubator/trace/ExtendedDefaultTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
import static org.assertj.core.api.Assertions.assertThat;

import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.trace.AbstractDefaultTracerTest;
import io.opentelemetry.api.testing.internal.AbstractDefaultTracerTest;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.api.trace.TracerProvider;
import org.junit.jupiter.api.Test;
Expand Down
21 changes: 21 additions & 0 deletions api/testing-internal/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
plugins {
id("otel.java-conventions")
}

description = "OpenTelemetry API Testing (Internal)"
otelJava.moduleName.set("io.opentelemetry.api.testing.internal")

dependencies {
api(project(":api:all"))

implementation(project(":testing-internal"))

implementation("com.linecorp.armeria:armeria-junit5")
implementation("org.assertj:assertj-core")
implementation("org.mockito:mockito-core")
}

// Skip OWASP dependencyCheck task on test module
dependencyCheck {
skip = true
}
7 changes: 5 additions & 2 deletions ...y/api/logs/AbstractDefaultLoggerTest.java → ...g/internal/AbstractDefaultLoggerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,23 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.api.logs;
package io.opentelemetry.api.testing.internal;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.Value;
import io.opentelemetry.api.logs.Logger;
import io.opentelemetry.api.logs.LoggerProvider;
import io.opentelemetry.api.logs.Severity;
import io.opentelemetry.context.Context;
import java.time.Instant;
import java.util.concurrent.TimeUnit;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link DefaultLogger}. */
/** Unit tests for No-op {@link Logger}. */
public abstract class AbstractDefaultLoggerTest {

protected abstract LoggerProvider getLoggerProvider();
Expand Down
17 changes: 15 additions & 2 deletions ...api/metrics/AbstractDefaultMeterTest.java → ...ng/internal/AbstractDefaultMeterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,28 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.api.metrics;
package io.opentelemetry.api.testing.internal;

import static io.opentelemetry.api.common.AttributeKey.stringKey;

import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.metrics.DoubleCounter;
import io.opentelemetry.api.metrics.DoubleGauge;
import io.opentelemetry.api.metrics.DoubleHistogram;
import io.opentelemetry.api.metrics.DoubleUpDownCounter;
import io.opentelemetry.api.metrics.LongCounter;
import io.opentelemetry.api.metrics.LongGauge;
import io.opentelemetry.api.metrics.LongHistogram;
import io.opentelemetry.api.metrics.LongUpDownCounter;
import io.opentelemetry.api.metrics.Meter;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.metrics.ObservableDoubleMeasurement;
import io.opentelemetry.api.metrics.ObservableLongMeasurement;
import io.opentelemetry.context.Context;
import io.opentelemetry.internal.testing.slf4j.SuppressLogger;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link DefaultMeter}. */
/** Unit tests for No-op {@link Meter}. */
@SuppressLogger()
public abstract class AbstractDefaultMeterTest {
private final Meter meter = getMeter();
Expand Down Expand Up @@ -245,6 +257,7 @@ void noopObservableDoubleUpDownCounter_doesNotThrow() {
}

@Test
@SuppressWarnings("NullAway")
void noopBatchCallback_doesNotThrow() {
meter.batchCallback(() -> {}, null);
}
Expand Down
17 changes: 13 additions & 4 deletions .../api/trace/AbstractDefaultTracerTest.java → ...g/internal/AbstractDefaultTracerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,27 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.api.trace;
package io.opentelemetry.api.testing.internal;

import static io.opentelemetry.api.common.AttributeKey.stringKey;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatCode;

import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanBuilder;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.api.trace.TraceFlags;
import io.opentelemetry.api.trace.TraceState;
import io.opentelemetry.api.trace.Tracer;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.Context;
import java.time.Instant;
import java.util.concurrent.TimeUnit;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.Test;

/** Unit tests for {@link DefaultTracer}. */
/** Unit tests for No-op {@link Tracer}. */
// Need to suppress warnings for MustBeClosed because Android 14 does not support
// try-with-resources.
@SuppressWarnings("MustBeClosedChecker")
Expand Down Expand Up @@ -56,6 +63,7 @@ void defaultSpanBuilderWithName() {
}

@Test
@SuppressWarnings("NullAway")
void spanContextPropagationExplicitParent() {
assertThat(
defaultTracer
Expand Down Expand Up @@ -109,8 +117,9 @@ void spanContextPropagation_fromContextThenNoParent() {
}

@Test
@SuppressWarnings("NullAway")
void doNotCrash_NoopImplementation() {
assertThatCode(
Assertions.assertThatCode(
() -> {
SpanBuilder spanBuilder = defaultTracer.spanBuilder(null);
spanBuilder.setSpanKind(null);
Expand Down
16 changes: 10 additions & 6 deletions ...emetry/api/AbstractOpenTelemetryTest.java → ...g/internal/AbstractOpenTelemetryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,25 @@
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.api;
package io.opentelemetry.api.testing.internal;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.OpenTelemetry;
import io.opentelemetry.api.logs.LoggerProvider;
import io.opentelemetry.api.metrics.MeterProvider;
import io.opentelemetry.api.trace.TracerProvider;
import io.opentelemetry.context.propagation.ContextPropagators;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

/** Unit tests for {@link OpenTelemetry}. */
/** Unit tests for No-op {@link OpenTelemetry}. */
public abstract class AbstractOpenTelemetryTest {

@BeforeAll
public static void beforeClass() {
GlobalOpenTelemetry.resetForTest();
Expand Down Expand Up @@ -88,16 +91,17 @@ void independentNonGlobalPropagators() {
@Test
void setThenSet() {
setOpenTelemetry();
assertThatThrownBy(() -> GlobalOpenTelemetry.set(getOpenTelemetry()))
Assertions.assertThatThrownBy(() -> GlobalOpenTelemetry.set(getOpenTelemetry()))
jack-berg marked this conversation as resolved.
Show resolved Hide resolved
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("GlobalOpenTelemetry.set has already been called")
.hasStackTraceContaining("setOpenTelemetry");
}

@Test
void getThenSet() {
assertThat(getGlobalOpenTelemetry()).isInstanceOf(DefaultOpenTelemetry.class);
assertThatThrownBy(() -> GlobalOpenTelemetry.set(getOpenTelemetry()))
assertThat(getGlobalOpenTelemetry().getClass().getName())
.isEqualTo("io.opentelemetry.api.DefaultOpenTelemetry");
Assertions.assertThatThrownBy(() -> GlobalOpenTelemetry.set(getOpenTelemetry()))
.isInstanceOf(IllegalStateException.class)
.hasMessageContaining("GlobalOpenTelemetry.set has already been called")
.hasStackTraceContaining("getGlobalOpenTelemetry");
Expand Down
1 change: 1 addition & 0 deletions settings.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ rootProject.name = "opentelemetry-java"
include(":all")
include(":api:all")
include(":api:incubator")
include(":api:testing-internal")
include(":bom")
include(":bom-alpha")
include(":context")
Expand Down
Loading