zaptest: testLogSpy doesn't have to be thread-safe #567
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
prashantv
approved these changes
Mar 26, 2018
Codecov Report
@@ Coverage Diff @@
## abg/test-logger #567 +/- ##
===================================================
+ Coverage 97.48% 97.49% +0.01%
===================================================
Files 40 40
Lines 2026 2035 +9
===================================================
+ Hits 1975 1984 +9
Misses 43 43
Partials 8 8
Continue to review full report at Codecov.
|
This expands the TestingT interface to a much larger subset of `testing.TB`. The following were left out: - `Error` and `Log` were left out in favor of `Errorf` and `Logf` - `Fatal*` methods were left out in favor of `Errorf` followed by `FailNow` - `Skip*` methods were left out because our test logger shouldn't be skipping tests - `Helper` was left out because not all supported verisons of Go have that
Zap's errorOutput is used exclusively to log internal errors. If this ever gets used in a test, something catastrophic happened and we should fail the test. This changes the logger returned by zaptest.NewLogger to implement this behavior by passing a `WriteSyncer` to `zap.ErrorOutput` that marks the test as failed upon being used. To test this, we set up a test logger and replace its core with one that will always fail to write.
This simplifies the implementation of testLogSpy by removing all thread-safety from it. It's only used from these specific tests and never concurrently.
abhinav
force-pushed
the
abg/thread-unsafe-spy
branch
from
66afe28
to
d7b6a16
Compare
billf
approved these changes
Apr 7, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This simplifies the implementation of testLogSpy by removing all
thread-safety from it. It's only used from these specific tests and
never concurrently.