Added unit tests in pkg/es/config #5806

Nabil-Salah · 2024-08-05T21:04:54Z

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

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

pkg/es/config/config_test.go

yurishkuro · 2024-08-05T21:31:37Z

the DCO check is failing. You'd be able to fix it at the end by squashing commits, but it would become more difficult if you start merging main into PR. It's best to get into habit of git commit -s.

yurishkuro

please fix DCO

pkg/es/config/config_test.go

Nabil-Salah · 2024-08-08T12:38:34Z

the DCO check is failing. You'd be able to fix it at the end by squashing commits, but it would become more difficult if you start merging main into PR. It's best to get into habit of git commit -s.

it passes now the DCO

yurishkuro

I think you messed up the merge, you have irrelevant changes in the pr.

codecov · 2024-08-08T17:14:36Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.81%. Comparing base (eaacbf0) to head (6ec1fb8).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5806      +/-   ##
==========================================
+ Coverage   96.65%   96.81%   +0.15%     
==========================================
  Files         342      342              
  Lines       16525    16525              
==========================================
+ Hits        15972    15998      +26     
+ Misses        363      340      -23     
+ Partials      190      187       -3

Flag	Coverage Δ
badger_v1	`8.05% <ø> (ø)`
badger_v2	`1.81% <ø> (ø)`
cassandra-3.x-v1	`16.61% <ø> (ø)`
cassandra-3.x-v2	`1.74% <ø> (ø)`
cassandra-4.x-v1	`16.61% <ø> (ø)`
cassandra-4.x-v2	`1.74% <ø> (ø)`
elasticsearch-6.x-v1	`18.77% <ø> (ø)`
elasticsearch-7.x-v1	`18.84% <ø> (ø)`
elasticsearch-8.x-v1	`19.03% <ø> (ø)`
elasticsearch-8.x-v2	`1.81% <ø> (ø)`
grpc_v1	`9.51% <ø> (ø)`
grpc_v2	`7.14% <ø> (ø)`
kafka-v1	`9.74% <ø> (ø)`
kafka-v2	`1.81% <ø> (ø)`
memory_v2	`1.81% <ø> (+0.01%)`	⬆️
opensearch-1.x-v1	`18.89% <ø> (+0.01%)`	⬆️
opensearch-2.x-v1	`18.88% <ø> (ø)`
opensearch-2.x-v2	`1.80% <ø> (ø)`
unittests	`95.29% <ø> (+0.21%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: nabil salah <nabil.salah203@gmail.com>

Nabil-Salah · 2024-08-08T17:36:17Z

I think you messed up the merge, you have irrelevant changes in the pr.

It's Now all fixed and the linter problem also

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us>

yurishkuro · 2024-08-08T18:26:39Z

sorry I had to revert this, didn't notice that it has the UI changes. Please redo as a new PR, from clean branch. Also, try removing ./pkg/es/config/.nocover, as this is the whole point of the main ticket and it's the last remaining .nocover file.

Nabil-Salah · 2024-08-08T19:17:11Z

sorry I had to revert this, didn't notice that it has the UI changes. Please redo as a new PR, from clean branch. Also, try removing ./pkg/es/config/.nocover, as this is the whole point of the main ticket and it's the last remaining .nocover file.

whe I removed .nocover I got no goleak check in package ./pkg/es/config/ is that ok?
after searching I found that I should put this line of code func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } and add it's go pkg but it still gives same error

yurishkuro · 2024-08-08T20:02:56Z

please create a new PR - yes we should add a TestMain for goleak check.

Nabil-Salah · 2024-08-08T20:16:59Z

please create a new PR - yes we should add a TestMain for goleak check.

the new pr: #5819

## 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us> 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us> 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us>

## 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us> 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us> 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>

This reverts commit aeb457a. The PR incorrectly include UI submodule changes. Signed-off-by: Yuri Shkuro <github@ysh.us>

Nabil-Salah requested a review from a team as a code owner August 5, 2024 21:04

Nabil-Salah requested a review from albertteoh August 5, 2024 21:04

dosubot bot added the storage/elasticsearch label Aug 5, 2024