chore: enable implicit default logger only in testing with -v

testcontainers / testcontainers-go

Testcontainers for Go is a Go package that makes it simple to create and clean up container-based dependencies for automated integration/smoke tests. The clean, easy-to-use API enables developers to programmatically define containers that should be run as part of a test and clean up those resources when the test is done.

https://golang.testcontainers.org

MIT License

3.67k stars 504 forks source link

chore: enable implicit default logger only in testing with -v #2877

Open apstndb opened 2 weeks ago

apstndb commented 2 weeks ago

What does this PR do?

This PR changes -v and -test.v=true handling only takes effect if testing.Testing() = true, i.e. go test.

Why is it important?

The current implementation always treats -v and -test.v=true specially, even outside of go test, and outputs verbose logs that are not easy to suppress. This makes it difficult to use testcontainers outside of go test.

Related issues

Closes #2874

How to test this PR

These behaviors should not be changed.

$ go test
$ go test -v
$ go test -test.v=true

These behavior will be changed to suppress default logging.

$ go run ./ -v

netlify[bot] commented 2 weeks ago

Deploy Preview for testcontainers-go ready!

Name	Link
Latest commit	a9fbef94b9994ee0e703f512032211dbbeca7f7a
Latest deploy log	https://app.netlify.com/sites/testcontainers-go/deploys/672b410bbe58800008652a73
Deploy Preview	https://deploy-preview-2877--testcontainers-go.netlify.app
Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mdelapenya commented 2 weeks ago

@apstndb thanks for opening the issue and working on the fix. I have one question about your use case: What do you expect when go run -v is executed?

From what I can see with the implementation in this PR, for the use case above of executing go run -v, we are going to always suppress the logging. I'd expect the -v flag enables the logging.

Could you share more on your use case so we can understand better the needs behind it? 🙏

apstndb commented 2 weeks ago

My expectation is that go run ./(or binary built by go build) won't generally follow go test convention.

It won't know -test.v=true.
It can use --verbose or -vv for vebose mode, and it can even use -v for other purpose.

It means the current behavior of enforcing go test convention to go run ./ is not good.

In this case, users can implement their own handling by explicitly setting testcontainers.Logger = logger or testcontainers.WithLogger(logger). It is not hard.

mdelapenya commented 3 days ago

@apstndb so IIUC, you are using testcontainers-go outside a test suite as, my guess, the runtime of a software that spins up containers, right?

Then I think you're right and we should consider this, but want to double check with you about your use case.

apstndb commented 3 days ago

so IIUC, you are using testcontainers-go outside a test suite as, my guess, the runtime of a software that spins up containers, right?

Yes! This is a combination of interactive tools and the Spanner emulator, so you could call it testing in the broadest sense, but it has nothing to do with the go test framework or unit testing.