Move Archive test into shared integration test suite

[Wise-Wizard]

Which problem is this PR solving?

This PR addresses the issue #5203

Description of the changes
This PR addresses the issue #5203 by integrating the testArchiveTrace function into the common StorageIntegration type. This modification ensures compatibility with multiple storage backends, allowing the archiving test to be executed across different environments.

How was this change tested?

The changes were tested by running the following command:

make test

Checklist

• I have read 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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.09%. Comparing base (2285143) to head (effeb3a).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5207      +/-   ##
==========================================
+ Coverage   95.06%   95.09%   +0.03%     
==========================================
  Files         340      340              
  Lines       16628    16652      +24     
==========================================
+ Hits        15808    15836      +28     
+ Misses        630      624       -6     
- Partials      190      192       +2     

Flag	Coverage Δ
badger	13.28% <20.83%> (+0.02%)	⬆️
cassandra-3.x	26.42% <20.83%> (-0.03%)	⬇️
cassandra-4.x	26.42% <20.83%> (-0.03%)	⬇️
elasticsearch-5.x	22.08% <91.66%> (+0.39%)	⬆️
elasticsearch-6.x	22.07% <91.66%> (+0.36%)	⬆️
elasticsearch-7.x	22.17% <91.66%> (+0.39%)	⬆️
elasticsearch-8.x	22.23% <91.66%> (+0.36%)	⬆️
grpc	11.54% <91.66%> (+0.55%)	⬆️
kafka	14.67% <0.00%> (-0.06%)	⬇️
opensearch-1.x	22.17% <91.66%> (+0.39%)	⬆️
opensearch-2.x	22.15% <91.66%> (+0.36%)	⬆️
unittests	92.27% <91.66%> (+0.02%)	⬆️

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.

[yurishkuro]

there should not be submodule changes in this PR

[Wise-Wizard]

there should not be submodule changes in this PR

Yeah, actually before making any changes at all. In the main branch after executing git pull, the submodule showed M (Modified) icon automatically. So, I thought those changes were coming from git pull itself?

[yurishkuro]

there should not be submodule changes in this PR

You can do this:

git checkout main
git pull 
git submodule update --recursive
git checkout pr-branch
git commit -sm 'fix submodules'

Your main branch must be up to date with upstream.

[yurishkuro]

where is the "extract" part of this PR? The ES storage integration test still contains archive logic.

[Wise-Wizard]

where is the "extract" part of this PR? The ES storage integration test still contains archive logic.

Yes, I am yet to remove the archive logic from ES Storage Integration. Thought will do that after the logic is approved.

[Wise-Wizard]

Hi, @yurishkuro. I had a query regarding this issue. You mentioned that the archive span reader/writer is not being ued. But, in the existing code of elasticsearch_test.go, the archive trace test is using the same Span Reader/Writer received from integration.go field. So, is that wrong itself?

[Wise-Wizard]

Hi, @yurishkuro. I had a query regarding this issue. You mentioned that the archive span reader/writer is not being ued. But, in the existing code of elasticsearch_test.go, the archive trace test is using the same Span Reader/Writer received from integration.go field. So, is that wrong itself?

Why can't I just extract this to integration.go, and add a condition to check if the specific storage supports archiving and then only execute the test?

[yurishkuro]

Why can't I just extract this to integration.go, and add a condition to check if the specific storage supports archiving and then only execute the test?

That was exactly the objective of the ticket, but it's not what your PR is doing. "extract" means "move", not duplicate.