-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Added missing tests for the go/streamlog package #15064
Added missing tests for the go/streamlog package #15064
Conversation
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following change has to be made to the tests.
go/streamlog/streamlog_test.go
Outdated
@@ -260,3 +264,152 @@ func TestFile(t *testing.T) { | |||
t.Errorf("streamlog file: want %q got %q", want, got) | |||
} | |||
} | |||
|
|||
func TestSetAndGetMethods(t *testing.T) { | |||
SetRedactDebugUIQueries(false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whenever we change the global state in a test, we should ensure we set it back in a defer function call.
So, we should add this code to the test -
originalVal := GetRedactDebugUIQueries()
defer func() {
SetRedactDebugUIQueries(originalVal)
}()
We should do this for all the tests that change any state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, thanks for the pointer.
ea167e7
to
cc5d0d4
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #15064 +/- ##
==========================================
+ Coverage 70.60% 70.62% +0.01%
==========================================
Files 1376 1376
Lines 182302 182302
==========================================
+ Hits 128721 128749 +28
+ Misses 53581 53553 -28 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TestRegisterStreamLogFlags
and TestSetAndGetMethods
don't seem very useful. Please remove those two tests, and we can merge the remaining two.
Alright, I'll update the code soon. |
Signed-off-by: VaibhavMalik4187 <[email protected]>
cc5d0d4
to
cd1f73d
Compare
Done |
Description
This commit increased the code coverage of the
go/streamlog
package to 93%.Related Issue(s)
Partially addresses: #14931
Checklist