Document Store test refactoring

[masci]

Related Issues

• related to Revisit tests #2811

Proposed Changes

Refactor the tests into a class hierarchy that reflects the Document Store classes.

Why?

• The pytest markers we use to select which tests should be collected is not manageable anymore: we recently found out that entire portions of tests were not executed because the right combination of markers was never met. The way pytest collect tests organized into classes will let us getting rid of the markers matrix.
• Computing coverage at the moment is beyond difficult

How

• The DocumentStoreBaseTestAbstract class contains the tests for the abstract methods in BaseDocumentStore. These tests receive an instance of a document store in form of the ds fixture.
• Classes deriving from DocumentStoreBaseTestAbstract are responsible for defining the ds fixture, returning the specific instance of the Document Store they intend to test
• pytest will run the testing code from DocumentStoreBaseTestAbstract for each subclass, hence exercising a different Document Store at each round

Notes for the reviewer

• This PR only touches the SearchEngineDocumentStore hierarchy, the other will follow in separate PRs
• The tests marked with pytest.mark.elasticsearch were moved in the new hierarchy
• Any other test in test_document_store.py was left there, waiting for the other Document Stores to be ported
• The Elasticsearch and Opensearch jobs in the workflow were replaced with new ones using the test classes

Checklist

• I have read the contributors guidelines and the code of conduct
• I have updated the related issue with new insights and changes
• I added tests that demonstrate the correct behavior of the change
• I've used the conventional commit convention for my PR title
• I documented my code
• I ran pre-commit hooks and fixed any issue

[vblagoje]

So as we add test_memory.py, it will get removed from these existing "references"? We'll do that one-by-one for each document store @masci ?

[masci]

@vblagoje correct, up to the point where we can remove test_document_store.py

[vblagoje]

Aha, this needs to match the test directory, we add a small comment perhaps?

[vblagoje]

We have test_opensearch.py, and we are removing OpenSearchDocumentStore from get_document_store yet we don't do the same for ElasticsearchDocumentStore. We are not removing it from get_document_store and we added 'test_elasticsearch.py'. Just a bit confused here. What's happening?

[masci]

Elasticsearch keeps being used in some tests in test_document_store.py that are triggered with the complex markers matrix, while Opensearch had a way smaller test coverage there. The result is that Opensearch tests are now 100% implemented in the new module test_opensearch.py while Elasticsearch needs more iterations to finish moving tests out of test_document_store.py into test_elasticsearch.py. This can't be done without touching the other Document Stores so I plan to do that later.

[vblagoje]

Now it looks gtg. Somewhat complex refactoring, but I see its intention, and it seems like a very good one!