.Net MEVD: Consider de-prioritizing/removing unit testing for MEVD connectors

[roji]

MEVD connectors currently have unit test projects in addition to integration testing. I'd propose shifting our focus to having a strong, reliable and shared suite of integration tests, and de-prioritize (and possibly even remove) the majority of unit tests.

• Database connectors are generally a place where integration tests seem much more useful than unit tests - they're relatively thin layers above the database, and generally have few complex internal components
• The coverage provided by unit tests is very partial. For example, the test can check that a connector generates some SQL string given a search, but that test doesn't verify that the SQL actually works, or returns the expected results.
• Integration tests (against the actual database) can be very reliable, with no flakiness - EF relies on integration tests for the vast majority of its testing (against multiple database types). Testcontainers take away the responsibility of waiting until a database is actually up, and some minimal logic (e.g. ensuring test data is actually visible after seeding) can ensure tests are fully reliable (see #10194 for more). Simply put, the time spent on unit tests is generally better spent on removing flakiness in integration tests (when flakiness occurs).
• Unit tests of internal connector components rely on internal API shapes, meaning that internal refactoring of a connector breaks them and causes more work. As an example, the work on LINQ-based filtering (#10273) needed to change some internal signatures, causing additional work to adapt the unit tests.
• Because integration tests work against the public surface APIs of MEVD, we can have general test suites that are used by all connectors (#10194). In contrast, unit tests must be written again and again for each connector, since the internal types being tested are different each time.
• Some connectors (e.g. Qdrant) even have a mock implementation in the connector itself - not the test project (MockableQdrantClient); this complicates the connector implementation and adds maintenance burden.
• One place where unit tests and mocking are important, is when testing various failure scenarios that can't be simulated in integration tests (e.g. network failure). However, these are generally more the concern of the low-level SDK/driver, whereas MEVD is a higher-level intermediate layer.

[dmytrostruk]

I still think that unit tests are useful in cases when you need to check the code which doesn't require a request to the database, for example in logic with SQL query generation or tests for telemetry. It's also easier to debug/troubleshoot your logic without starting a new Docker instance every time.

But I do agree that we should not spend a lot of time on it, and we should keep it as simple as possible. Ideally, we should leverage AI to generate unit tests for us whenever it's possible.

[roji]

for example in logic with SQL query generation

This is an important point - I think a unit test that simply asserts that a certain SQL has been generated has very low value (in fact negative), since it doesn't in any way confirm that the SQL actually works on the database, or does what it's supposed to do; I've seen many cases where tests were green and the SQL was as expected, but when you actually ran the thing it failed, because the SQL was simply wrong. So for any actual coverage, there would also have to be an end-to-end test involving the database, verifying actual behavior; and at that point, a unit test asserting on the SQL basically has zero value.

for telemetry

Right, I think we agree on this. With our current approach to telemetry (IIUC), we'd be adding a wrapping layer above a connector (OpenTelemetryVectorStore) which adds the telemetry. This would indeed be an isolated component that does not, in itself, contact any external database, so I agree with you that it can and should be tested in isolation. In other words, it makes sense here to pass a mock/fake implementation of IVectorStore as the wrapped connector to OpenTelemetryVectorStore, and test it like that.

So yeah, I agree that unit tests are the way to go for that kind of component (e.g. OpenTelemetryVectorStore) - because that's not an actual connector (but rather a wrapper).

It's also easier to debug/troubleshoot your logic without starting a new Docker instance every time.

I've been working with the new integration tests using testcontainers, and in general it feels to me that it starts up quickly enough so as not to interfere with debugging. But if you think that's a concern, we can also add the option of specifying an endpoint via an environment, which, if defined, is used instead of spinning up a container. This way, you can install e.g. PostgreSQL locally, and the PG integration tests run against that.

That still means you need to install the various services (PG, Redis...), at least for those where docker startup is slow (again, I haven't seen that really); and it's also impossible for hosted-only services (like Azure AI Search). But the point is that we need proper, end-to-end coverage of how the connectors behave, and unit tests simply don't provide that (while simultaneously adding maintenance burden, depend on internal API shapes, etc. etc.)

Ideally, we should leverage AI to generate unit tests for us whenever it's possible.

Definitely not against using AI here, but just pointing out that any test in the code base adds maintenance, and unit tests are particularly problematic since they assert on internal API shapes (so the connector becomes more convoluted internally to allow for unit testing, plus refactoring becomes hard).

My personal preferred view of what to do with the memory connector tests going forward:

• Redo/move all integration tests from the IntegrationTests project into the new VectorDataIntegrationTests
• Go over the unit tests, and ensure that what they exercise is also covered end-to-end in integration testing - otherwise we're lacking real coverage. That would mean porting over some unit tests to become integration tests.
  • Of course, some tests simply don't require database access (e.g. GetCollection()), though with a database connector these are going to be very few, and not very interesting.
• Ideally in my mind, we end up with a single test project for each connector, which covers the maximum behavior possible via end-to-end testing, and via inherited tests that apply to all connectors. There may be a handful of unit tests which remain for things that don't involve the database (and we can rename the project to remove the word "integration"), but those will generally be the exception and not very important.

Of course, all this is subject to discussion and none of this is urgent...