Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

Import of "testing" package #585

Closed
aaronbee opened this issue May 24, 2021 · 4 comments · Fixed by #586
Closed

Import of "testing" package #585

aaronbee opened this issue May 24, 2021 · 4 comments · Fixed by #586
Labels

Comments

@aaronbee
Copy link

Describe the bug
Importing jaeger-client-go adds a dependency on the standard library "testing" package. This is undesirable because production Go programs should not depend on the testing package. In addition to unnecessary bloat, "testing" adds a bunch of command line flags.

Here is the dependency graph that leads to "testing":

github.com/uber/jaeger-client-go -> 
github.com/uber/jaeger-client-go/utils ->
github.com/uber/jaeger-client-go/thrift-gen/jaeger ->
github.com/uber/jaeger-client-go/thrift -> 
testing

jaeger-client-go/thrift/logger.go is the file that imports "testing"

To Reproduce
go list -deps github.com/uber/jaeger-client-go | grep '^testing$'

Expected behavior
jaeger-client-go should not depend on "testing".

Version (please complete the following information):
Found in v2.29.0, not found in v2.28.0.

@yurishkuro
Copy link
Member

Added a fix in #586.

The dependency comes from vendored Thrift, so I opened a ticket there too: https://issues.apache.org/jira/browse/THRIFT-5420

@yurishkuro
Copy link
Member

@yurishkuro
Copy link
Member

@aaronbee could you elaborate on "adds a bunch of command line flags"? How does this manifest?

@aaronbee
Copy link
Author

Thanks for the fix.

I was mistaken on the flags issue. In earlier versions of Go if you had an import of "testing" it would register a bunch of test-specific flags so that your program's help output would include things like:

  -test.bench regexp
    	run only benchmarks matching regexp
  -test.benchmem
    	print memory allocations for benchmarks
  -test.benchtime d
    	run each benchmark for duration d (default 1s)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants