Skip to content

Commit

Permalink
Don't import testing package in production builds (#2786)
Browse files Browse the repository at this point in the history
* switch all tests to be on the global package

* move ResetForTest to be available only during tests

Co-authored-by: Chester Cheung <cheung.zhy.csu@gmail.com>
  • Loading branch information
dmathieu and hanyuancheung committed Apr 14, 2022
1 parent 46a10bb commit c65c3be
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 50 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm

- Delegated instruments are unwrapped before delegating Callbacks. (#2784)
- Resolve supply-chain failure for the markdown-link-checker GitHub action by calling the CLI directly. (#2834)
- Remove import of `testing` package in non-tests builds. (#2786)

## [0.29.0] - 2022-04-11

Expand Down
9 changes: 3 additions & 6 deletions internal/global/benchmark_test.go
Expand Up @@ -12,21 +12,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package global_test
package global

import (
"context"
"testing"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/internal/global"
)

func BenchmarkStartEndSpanNoSDK(b *testing.B) {
// Compare with BenchmarkStartEndSpan() in
// ../../sdk/trace/benchmark_test.go.
global.ResetForTest(b)
t := otel.Tracer("Benchmark StartEndSpan")
ResetForTest(b)
t := TracerProvider().Tracer("Benchmark StartEndSpan")
ctx := context.Background()
b.ResetTimer()
for i := 0; i < b.N; i++ {
Expand Down
23 changes: 11 additions & 12 deletions internal/global/propagator_test.go
Expand Up @@ -12,23 +12,22 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package global_test
package global

import (
"context"
"testing"

"go.opentelemetry.io/otel/internal/global"
"go.opentelemetry.io/otel/internal/internaltest"
)

func TestTextMapPropagatorDelegation(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)
ctx := context.Background()
carrier := internaltest.NewTextMapCarrier(nil)

// The default should be a noop.
initial := global.TextMapPropagator()
initial := TextMapPropagator()
initial.Inject(ctx, carrier)
ctx = initial.Extract(ctx, carrier)
if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) {
Expand All @@ -45,28 +44,28 @@ func TestTextMapPropagatorDelegation(t *testing.T) {

// The initial propagator should use the delegate after it is set as the
// global.
global.SetTextMapPropagator(delegate)
SetTextMapPropagator(delegate)
initial.Inject(ctx, carrier)
ctx = initial.Extract(ctx, carrier)
delegate.InjectedN(t, carrier, 2)
delegate.ExtractedN(t, ctx, 2)
}

func TestTextMapPropagatorDelegationNil(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)
ctx := context.Background()
carrier := internaltest.NewTextMapCarrier(nil)

// The default should be a noop.
initial := global.TextMapPropagator()
initial := TextMapPropagator()
initial.Inject(ctx, carrier)
ctx = initial.Extract(ctx, carrier)
if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) {
return
}

// Delegation to nil should not make a change.
global.SetTextMapPropagator(nil)
SetTextMapPropagator(nil)
initial.Inject(ctx, carrier)
initial.Extract(ctx, carrier)
if !carrier.GotN(t, 0) || !carrier.SetN(t, 0) {
Expand All @@ -75,22 +74,22 @@ func TestTextMapPropagatorDelegationNil(t *testing.T) {
}

func TestTextMapPropagatorFields(t *testing.T) {
global.ResetForTest(t)
initial := global.TextMapPropagator()
ResetForTest(t)
initial := TextMapPropagator()
delegate := internaltest.NewTextMapPropagator("test")
delegateFields := delegate.Fields()

// Sanity check on the initial Fields.
if got := initial.Fields(); fieldsEqual(got, delegateFields) {
t.Fatalf("testing fields (%v) matched Noop fields (%v)", delegateFields, got)
}
global.SetTextMapPropagator(delegate)
SetTextMapPropagator(delegate)
// Check previous returns from global not correctly delegate.
if got := initial.Fields(); !fieldsEqual(got, delegateFields) {
t.Errorf("global TextMapPropagator.Fields returned %v instead of delegating, want (%v)", got, delegateFields)
}
// Check new calls to global.
if got := global.TextMapPropagator().Fields(); !fieldsEqual(got, delegateFields) {
if got := TextMapPropagator().Fields(); !fieldsEqual(got, delegateFields) {
t.Errorf("global TextMapPropagator.Fields returned %v, want (%v)", got, delegateFields)
}
}
Expand Down
12 changes: 0 additions & 12 deletions internal/global/state.go
Expand Up @@ -18,7 +18,6 @@ import (
"errors"
"sync"
"sync/atomic"
"testing"

"go.opentelemetry.io/otel/propagation"
"go.opentelemetry.io/otel/trace"
Expand Down Expand Up @@ -114,14 +113,3 @@ func defaultPropagatorsValue() *atomic.Value {
v.Store(propagatorsHolder{tm: newTextMapPropagator()})
return v
}

// ResetForTest configures the test to restores the initial global state during
// its Cleanup step
func ResetForTest(t testing.TB) {
t.Cleanup(func() {
globalTracer = defaultTracerValue()
globalPropagators = defaultPropagatorsValue()
delegateTraceOnce = sync.Once{}
delegateTextMapPropagatorOnce = sync.Once{}
})
}
38 changes: 18 additions & 20 deletions internal/global/trace_test.go
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

package global_test
package global

import (
"context"
Expand All @@ -22,8 +22,6 @@ import (

"github.com/stretchr/testify/assert"

"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/internal/global"
"go.opentelemetry.io/otel/trace"
)

Expand All @@ -44,7 +42,7 @@ func (fn fnTracer) Start(ctx context.Context, spanName string, opts ...trace.Spa
}

func TestTraceProviderDelegation(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

// Map of tracers to expected span names.
expected := map[string][]string{
Expand All @@ -54,12 +52,12 @@ func TestTraceProviderDelegation(t *testing.T) {
}

ctx := context.Background()
gtp := otel.GetTracerProvider()
gtp := TracerProvider()
tracer1 := gtp.Tracer("pre")
// This is started before an SDK was registered and should be dropped.
_, span1 := tracer1.Start(ctx, "span1")

otel.SetTracerProvider(fnTracerProvider{
SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
spans, ok := expected[name]
assert.Truef(t, ok, "invalid tracer: %s", name)
Expand Down Expand Up @@ -98,14 +96,14 @@ func TestTraceProviderDelegation(t *testing.T) {
}

func TestTraceProviderDelegates(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

// Retrieve the placeholder TracerProvider.
gtp := otel.GetTracerProvider()
gtp := TracerProvider()

// Configure it with a spy.
called := false
otel.SetTracerProvider(fnTracerProvider{
SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
called = true
assert.Equal(t, "abc", name)
Expand All @@ -118,10 +116,10 @@ func TestTraceProviderDelegates(t *testing.T) {
}

func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

// Retrieve the placeholder TracerProvider.
gtp := otel.GetTracerProvider()
gtp := TracerProvider()

done := make(chan struct{})
quit := make(chan struct{})
Expand All @@ -142,7 +140,7 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) {

// Configure it with a spy.
called := int32(0)
otel.SetTracerProvider(fnTracerProvider{
SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
newVal := atomic.AddInt32(&called, 1)
assert.Equal(t, "abc", name)
Expand All @@ -161,10 +159,10 @@ func TestTraceProviderDelegatesConcurrentSafe(t *testing.T) {
}

func TestTracerDelegatesConcurrentSafe(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

// Retrieve the placeholder TracerProvider.
gtp := otel.GetTracerProvider()
gtp := TracerProvider()
tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))

done := make(chan struct{})
Expand All @@ -186,7 +184,7 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) {

// Configure it with a spy.
called := int32(0)
otel.SetTracerProvider(fnTracerProvider{
SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
assert.Equal(t, "abc", name)
return fnTracer{
Expand All @@ -210,15 +208,15 @@ func TestTracerDelegatesConcurrentSafe(t *testing.T) {
}

func TestTraceProviderDelegatesSameInstance(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

// Retrieve the placeholder TracerProvider.
gtp := otel.GetTracerProvider()
gtp := TracerProvider()
tracer := gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz"))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))
assert.Same(t, tracer, gtp.Tracer("abc", trace.WithInstrumentationVersion("xyz")))

otel.SetTracerProvider(fnTracerProvider{
SetTracerProvider(fnTracerProvider{
tracer: func(name string, opts ...trace.TracerOption) trace.Tracer {
return trace.NewNoopTracerProvider().Tracer("")
},
Expand All @@ -228,7 +226,7 @@ func TestTraceProviderDelegatesSameInstance(t *testing.T) {
}

func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) {
global.ResetForTest(t)
ResetForTest(t)

sc := trace.NewSpanContext(trace.SpanContextConfig{
TraceID: [16]byte{0x01},
Expand All @@ -237,7 +235,7 @@ func TestSpanContextPropagatedWithNonRecordingSpan(t *testing.T) {
Remote: true,
})
ctx := trace.ContextWithSpanContext(context.Background(), sc)
_, span := otel.Tracer("test").Start(ctx, "test")
_, span := TracerProvider().Tracer("test").Start(ctx, "test")

assert.Equal(t, sc, span.SpanContext())
assert.False(t, span.IsRecording())
Expand Down
31 changes: 31 additions & 0 deletions internal/global/util_test.go
@@ -0,0 +1,31 @@
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package global

import (
"sync"
"testing"
)

// ResetForTest configures the test to restores the initial global state during
// its Cleanup step
func ResetForTest(t testing.TB) {
t.Cleanup(func() {
globalTracer = defaultTracerValue()
globalPropagators = defaultPropagatorsValue()
delegateTraceOnce = sync.Once{}
delegateTextMapPropagatorOnce = sync.Once{}
})
}

0 comments on commit c65c3be

Please sign in to comment.