Extract archive storage integration test from ES and generalize to all storage backends #5203
Labels
good first issue
Good for beginners
help wanted
Features that maintainers are willing to accept but do not have cycles to implement
Comments
yurishkuro
added
help wanted
|
Should I create a new test file under integration itself, to test archiving for all Backends? Or can I just add the tests to the pre-existing test files for each storage? |
|
we already have integration.go as the shared framework for all backends, it should go there |
|
Understood... |
Closed
4 tasks
4 tasks
yurishkuro
added a commit
that referenced
this issue
Mar 29, 2024
**Which problem is this PR solving?** This PR addresses the issue [#5203 ](#5203) **Description of the changes** This PR addresses the issue [#5203](#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: ```bash make test ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: Wise-Wizard <saransh.shankar@gmail.com> Signed-off-by: Saransh Shankar <103821431+Wise-Wizard@users.noreply.github.com> Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>
4 tasks
yurishkuro
added a commit
that referenced
this issue
Apr 6, 2024
**Which problem is this PR solving?** This PR addresses a new found issue which is a part of the issue [#5203 ](#5203). It initializes ES Factory in the Integration Test Suite. Description: This PR introduces changes to the integration tests by initializing an ElasticSearch factory (es.Factory) and utilizing it for the creation of span readers and writers. Previously, the code instantiated a new span reader/writer from the SpanStore directly, which has been replaced with the more appropriate approach of leveraging the ElasticSearch factory. **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: Wise-Wizard <saransh.shankar@gmail.com> Signed-off-by: Yuri Shkuro <github@ysh.us> Co-authored-by: Yuri Shkuro <github@ysh.us>
4 tasks
|
Hi, I believe the Archive Trace test functionality is only remaining for Cassandra and Badger after my PR. Should I continue working on them and close this issue? |
|
@yurishkuro, may I pick up on this? |
4 tasks
yurishkuro
pushed a commit
that referenced
this issue
May 8, 2024
**Which problem is this PR solving?** This PR addresses the issue [#5203 ](#5203) **Description of the changes** This PR addresses the issue [#5203](#5203) by integrating the testArchiveTrace function into the common Storage Integration type. Added the Archive Trace Test functionality to remaining Backends including Cassandra and Badger **How was this change tested?** The changes were tested by running the following command: ```bash make test ``` ```bash bash scripts/cassandra-integration-test.sh ``` **Checklist** - [x] I have read [CONTRIBUTING_GUIDELINES.md](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: Wise-Wizard <saransh.shankar@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I just noticed, to my surprise, that archiving test is only implemented in the ES integration tests
jaeger/plugin/storage/integration/elasticsearch_test.go
Line 295 in 8ae2e05
Objective
The text was updated successfully, but these errors were encountered: