Implement missing unit tests #5068

yurishkuro · 2024-01-02T18:42:09Z

Our CI enforces that all packages with .go files must have at least one _test.go file, to ensure the package is included in the code coverage, OR that it has a .nocover file when testing it is very difficult (e.g. requires external DB connection).

However, we somewhat abused the .nocover capability and have a bunch of packages that could have tests with high coverage. This ticket is about incrementally fixing those, which is a good first issue since the changes can be reasonably small if scoped to one package.

To see which packages need these tests, run make nocover.

Regarding ./examples/, where lots of .nocover files are found today, I think we can replace them with empty_test.go as we do in some other dirs, because examples will still be excluded from the overall coverage percentage, so there's no need to be excluding them via .nocover mechanism (it just creates noise).

Update Jun 17 2024

We are actually down to a single package remaining

$ make nocover
Verifying that all packages have test files to count in coverage
Package excluded from coverage: ./pkg/es/config/
  reason: requires connection to Elasticsearch

The reason stated in that package is not very valid - it only checks for the version of the server which can be easily mocked with a simple HTTP server as we already do in other ES-related tests. The majority of methods in the package do not require connection at all and can be 100% covered by tests.

The text was updated successfully, but these errors were encountered:

chirag-ghosh · 2024-01-03T04:27:16Z

I would like to work on this

yurishkuro · 2024-01-03T04:28:48Z

@chirag-ghosh thanks. I suggest you pick one of the packages and maybe post here that you're working on it, in case someone else also wants to pick another package. Don't combine multiple packages in one PR.

chirag-ghosh · 2024-01-03T04:34:05Z

Sure. Since I'm new to the project, I'll first setup it in my local and the comment the package I'm working on ASAP.

chirag-ghosh · 2024-01-03T05:14:02Z

I am working on the storageexporter package (https://github.com/jaegertracing/jaeger/tree/main/cmd/jaeger/internal/exporters/storageexporter)

akagami-harsh · 2024-01-03T05:53:32Z

working on cmd/jeager/internal

Pushkarm029 · 2024-01-03T06:14:39Z

working on ./pkg/version/ & adding empty tests for ./examples/

## Which problem is this PR solving? - Part of #5068 ## Description of the changes - added some missing unit tests in /pkg/version ## How was this change tested? - go test ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Pushkar Mishra <pushkarmishra029@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? - Part of #5068 ## Description of the changes - Added Empty Tests for `./examples` to reduce noise in `make nocover`. ## How was this change tested? - go test ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Pushkar Mishra <pushkarmishra029@gmail.com>

## Which problem is this PR solving? - Part of #5068 ## Description of the changes - Add tests to storageexporter package ## How was this change tested? - CI ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Chirag Ghosh <cghosh828049@gmail.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <github@ysh.us>

chirag-ghosh · 2024-01-10T18:22:18Z

Working on jaegerstorage

tarhphilomina · 2024-01-12T12:20:35Z

working on Jaeger/plugin/

## Which problem is this PR solving? Part of #5068 ## Description of the changes - Added test to adaptive sampling plugin options ## How was this change tested? - make test ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: tarhphilomina <156124227+tarhphilomina@users.noreply.github.com>

## Which problem is this PR solving? - #5068 ## Description of the changes - Add unit tests for jaegerstorage ## How was this change tested? - ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Chirag Ghosh <cghosh828049@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? - part of #5068 ## Description of the changes - added missing unit test in cmd/jeager/internal and also added a goroutine check for that package ## How was this change tested? - go test ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` --------- Signed-off-by: Harshvir Potpose <hpotpose62@gmail.com> Signed-off-by: Harshvir Potpose <122517264+akagami-harsh@users.noreply.github.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Yuri Shkuro <github@ysh.us>

VaibhavMalik4187 · 2024-01-19T06:32:27Z

I'd like to work on internal/tracegen

- part of jaegertracing#5068 - This commit adds tests for the `Run` function defined in the `internal/tracegen` package. - make test - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` --------- Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>

- Partially Fixes jaegertracing#5068 - This commit adds tests for the `Run` function defined in the `internal/tracegen` package. - make test - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` --------- Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>

- Partially fixes jaegertracing#5068 - This commit adds tests for the `Run` function defined in the `internal/tracegen` package. - make test - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com>

VaibhavMalik4187 · 2024-01-19T17:17:24Z

I'm working on the cmd/jaeger/internal/extension/jaegerquery/ package.

Wise-Wizard · 2024-01-31T04:11:10Z

Hi @VaibhavMalik4187, do you plan on completing the PR regarding tests for JaegerQuery or I can go ahead and complete the remaining requirements?

h4shk4t · 2024-02-03T09:34:49Z

I'd like to work on cmd/anonymizer/app/writer/

miledxz · 2024-02-05T14:55:43Z

Heey @yurishkuro, I can add tests too, what do u suggest is priority at the moment ?

## Which problem is this PR solving? - Part of [#5068](#5068) ## Description of the changes - Added unit tests for `cmd/anonymizer/app/writer/` ## How was this change tested? - `go test -cover -v ./cmd/anonymizer/app/writer` - `make lint test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Ashutosh Srivastava <ashutosh3002@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? - Part of #5068 ## Description of the changes - This commit adds tests for the `cmd/jaeger/internal/extension/jaegerquery` package. ## How was this change tested? - make test ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: VaibhavMalik4187 <vaibhavmalik2018@gmail.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Signed-off-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Co-authored-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? - Part of #5068 ## Description of the changes - This commit adds tests for the cmd/anonymizer/app/query package. ## How was this change tested? - `go test ./cmd/anonymizer/app/query -v -cover` (85.7%) ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: Pratham <prathms007@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? - Part of #5068 - Looks like test were added but `.nocover` files not removed. ## Description of the changes - Remove unnecessary .nocover files. ## How was this change tested? - Check if this affects coverage in CI. --------- Signed-off-by: Yuri Shkuro <github@ysh.us>

## Which problem is this PR solving? - #5068 ## Description of the changes - added test case for ` pkg/kafka/auth/config.go` and updated `internal/tracegen/worker_test.go` ## How was this change tested? - ## Checklist - [ ] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [ ] I have signed all commits - [ ] I have added unit tests for the new functionality - [ ] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` --------- Signed-off-by: mehul gautam <mehulsharma4786@gmail.com> Signed-off-by: mehul gautam <mehulsharma4786@gamil.com> Co-authored-by: mehul gautam <mehulsharma4786@gamil.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? partially fixes: #5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? Part of: #5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com>

yurishkuro · 2024-08-08T22:22:49Z

This is DONE! 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉 🎉

Thanks to everyone who contributed: @akagami-harsh @Pushkarm029 @chirag-ghosh @tarhphilomina @VaibhavMalik4187 @Wise-Wizard @h4shk4t @shanukun @hellspawn679 @Nabil-Salah

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? Part of: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? Part of: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? Part of: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com> Signed-off-by: Jared Tan <jian.tan@daocloud.io>

## Which problem is this PR solving? partially fixes: jaegertracing#5068 ## Description of the changes - This commit adds tests for the `pkg/es/config` package. ## How was this change tested? - `make test` ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `yarn lint` and `yarn test` Signed-off-by: nabil salah <nabil.salah203@gmail.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Jan 2, 2024

akagami-harsh mentioned this issue Jan 3, 2024

Add unit tests in cmd/jaeger/internal #5069

Merged

4 tasks

Pushkarm029 mentioned this issue Jan 3, 2024

Added unit tests for /pkg/version #5074

Merged

4 tasks

yurishkuro added a commit to Pushkarm029/jaeger that referenced this issue Jan 3, 2024

Merge branch 'main' into jaegertracing#5068

874d426

Pushkarm029 mentioned this issue Jan 4, 2024

Added Empty Tests for Examples #5076

Merged

4 tasks

chirag-ghosh mentioned this issue Jan 4, 2024

Add test for storage-exporter #5078

Merged

4 tasks

chirag-ghosh mentioned this issue Jan 12, 2024

Add tests for cmd/jaeger/internal/extension/jaegerstorage #5096

Merged

4 tasks

tarhphilomina mentioned this issue Jan 15, 2024

Added test on plugin/sampling/strategystore/adaptive/options #5105

Merged

4 tasks

VaibhavMalik4187 mentioned this issue Jan 19, 2024

Added unit tests in internal/tracegen #5119

Merged

4 tasks

VaibhavMalik4187 mentioned this issue Jan 19, 2024

Add tests for internal/extension/jaegerquery #5123

Merged

4 tasks

h4shk4t mentioned this issue Feb 4, 2024

Add unit tests for anonymizer/app/writer #5162

Merged

4 tasks

shanukun mentioned this issue Jun 16, 2024

Add tests for anonymizer/app/query. #5638

Merged

4 tasks

yurishkuro mentioned this issue Jun 17, 2024

Remove unnecessary .nocover files #5642

Merged

hellspawn679 mentioned this issue Jun 27, 2024

Increase codecov of pkg/kafka #5682

Merged

4 tasks

This was referenced Aug 4, 2024

implement ./pkg/es/config/ tests #5801

Closed

Added unit tests in pkg/es/config #5806

Merged

yurishkuro closed this as completed in #5806 Aug 8, 2024

yurishkuro reopened this Aug 8, 2024

Nabil-Salah mentioned this issue Aug 8, 2024

Added unit tests in pkg/es/config #5819

Merged

4 tasks

yurishkuro closed this as completed Aug 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement missing unit tests #5068

Implement missing unit tests #5068

yurishkuro commented Jan 2, 2024 •

edited

Loading

chirag-ghosh commented Jan 3, 2024

yurishkuro commented Jan 3, 2024

chirag-ghosh commented Jan 3, 2024

chirag-ghosh commented Jan 3, 2024

akagami-harsh commented Jan 3, 2024

Pushkarm029 commented Jan 3, 2024 •

edited

Loading

chirag-ghosh commented Jan 10, 2024

tarhphilomina commented Jan 12, 2024

VaibhavMalik4187 commented Jan 19, 2024

VaibhavMalik4187 commented Jan 19, 2024 •

edited

Loading

Wise-Wizard commented Jan 31, 2024

h4shk4t commented Feb 3, 2024 •

edited

Loading

miledxz commented Feb 5, 2024

yurishkuro commented Aug 8, 2024

Implement missing unit tests #5068

Implement missing unit tests #5068

Comments

yurishkuro commented Jan 2, 2024 • edited Loading

Update Jun 17 2024

chirag-ghosh commented Jan 3, 2024

yurishkuro commented Jan 3, 2024

chirag-ghosh commented Jan 3, 2024

chirag-ghosh commented Jan 3, 2024

akagami-harsh commented Jan 3, 2024

Pushkarm029 commented Jan 3, 2024 • edited Loading

chirag-ghosh commented Jan 10, 2024

tarhphilomina commented Jan 12, 2024

VaibhavMalik4187 commented Jan 19, 2024

VaibhavMalik4187 commented Jan 19, 2024 • edited Loading

Wise-Wizard commented Jan 31, 2024

h4shk4t commented Feb 3, 2024 • edited Loading

miledxz commented Feb 5, 2024

yurishkuro commented Aug 8, 2024

yurishkuro commented Jan 2, 2024 •

edited

Loading

Pushkarm029 commented Jan 3, 2024 •

edited

Loading

VaibhavMalik4187 commented Jan 19, 2024 •

edited

Loading

h4shk4t commented Feb 3, 2024 •

edited

Loading