Add unit tests in cmd/jaeger/internal #5069
Conversation
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5069 +/- ##
========================================
Coverage 95.58% 95.59%
========================================
Files 320 322 +2
Lines 18347 18454 +107
========================================
+ Hits 17537 17641 +104
- Misses 651 653 +2
- Partials 159 160 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
i tried adding this test in func TestCommandNoConfigFlag(t *testing.T) {
cmd := Command()
cmd.SetArgs([]string{})
err := cmd.Execute()
require.Error(t, err, "Expected an error when no config flag is provided")
assert.Contains(t, err.Error(), "some error related to missing config", "Error message should indicate missing configuration")
}to increase code coverage, but it was failing |
There was a problem hiding this comment.
to increase code coverage, but it was failing goleak somehow.
you can investigate which ones are leaking. In the current test you don't even run the command, so it's surprising that it leaks, but to actually achieve code coverage you will need to execute the command.
There was a problem hiding this comment.
$ go test -cover ./cmd/jaeger/internal/
ok github.com/jaegertracing/jaeger/cmd/jaeger/internal 0.434s coverage: 60.0% of statements
Package coverage needs to be at the level of overall project (95%+).
The code in command.go can be tested if L42 is moved to a function that can be called from a test directly and substitute cmd.RunE
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
yurishkuro
added
the
changelog:test
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
There was a problem hiding this comment.
What's remaining is the error paths in the components() function. The way to test them is to add a helper function that takes a bunch of functions that components() currently calls, like extension.MakeFactoryMap, etc., and mock them in the test to return errors one at a time.
Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Harshvir Potpose <[email protected]>
Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Harshvir Potpose <[email protected]>
Signed-off-by: Yuri Shkuro <[email protected]>
| return func(factories ...FACTORY) (map[component.Type]FACTORY, error) { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
use template to remove duplication
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint test