DRAFT: test(early implementation): add abstract test class for vector stores with local and…

[sergerdn]

DRAFT

There have been no changes made to the current functionality of the tests or anything else. The only addition I've made is a draft for the future implementation of testing vector stores to ensure that they are thoroughly tested.

I've created the AbstractVectorStoreTestLocal class to test vector stores that can be either local with a persistent database on a file system or stored in memory.

Fix #2491

Also take a look #2816

[sergerdn]

@hwchase17

I am ready for review.

[hwchase17]

i think would be helpful to see this implement for 1 or 2 vectorstores, to see if abstractions are correct or not

i also wonder if this could just be a bunch of tests, parameterized by a list of vectorstore classes. eg does each vectorstore need its own testing class, or should we just one one testing function for semantic search, which is called in a loop on all vectorstores

[sergerdn]

i think would be helpful to see this implement for 1 or 2 vectorstores, to see if abstractions are correct or not

Agree.

i also wonder if this could just be a bunch of tests, parameterized by a list of vectorstore classes. eg does each vectorstore need its own testing class, or should we just one one testing function for semantic search, which is called in a loop on all vectorstores

An abstract class and another class are being used for testing. All TestVectorStore classes inherit from both classes, but only for specific preparatory tasks. I want to simplify this while ensuring that it's done correctly.

In any case, I believe that we only need one class to handle all the necessary work.

I am thinking about something like this:

class BaseVectorStoreTest(AbstractVectorStoreTest, ABC):
    """Doing all hard work here"""
    vector_store_class: Type[VectorStore]


class TestChromaRemote(AbstractVectorStoreTestRemote, BaseVectorStoreTest, ABC):
    vector_store_class = Chroma


  def setup_class(self) -> None:
      """Prepare the test environment to connect to the vector store"""


class TestChromaLocal(AbstractVectorStoreTestLocal, BaseVectorStoreTest, ABC):
    vector_store_class = Chroma

    def setup_class(self,db_store_dir) -> None:
        """Prepare the test environment to prepare local DB for the vector store"""

[sergerdn]

I have decided to move the new tests to a new directory. I believe that this will make it easier for me and anyone else to work with them. I am not sure that I will be able to implement tests for all vector stores at this moment. Once all the work is finished (all tests will be implemented with new api), the old directory can be removed.

If someone decides to change some tests during my work, we won't have a merge conflict in this case.

I want to make it as much better as I can, it needs more time for thinking rather than realisation.

If anyone, including you, has some good ideas for me, it will help me very much.

[sergerdn]

@hwchase17

I am ready for a review, but I haven't finished yet. I would appreciate some feedback from you.

[sergerdn]

pytest-dev/pyfakefs#813

[leo-gan]

@sergerdn Hi , could you, please, resolve the merging issues and address the last comments (if needed)? After that ping me and I push this PR for the review. Thanks!

If this PR is not needed anymore, could you, please, let me know?