Skip to content

[partial][draft][in-progress] migrating unit tests#928

Draft
JordanMaples wants to merge 1 commit intomainfrom
jordanmaples/migrate_unittests
Draft

[partial][draft][in-progress] migrating unit tests#928
JordanMaples wants to merge 1 commit intomainfrom
jordanmaples/migrate_unittests

Conversation

@JordanMaples
Copy link
Copy Markdown

@JordanMaples JordanMaples commented Apr 8, 2026

This is a first step in the effort to migrate unit tests from diskann_async to diskann #927. If you don't think tests that have been brought over should be moved, they probably shouldn't have been. Please point them out and I'll do my best to put them back where I found them.

I'll update this field as I continue to work on it:

  • Asked copilot to isolate trivially migratable tests and move them. It landed on the Multi-Hop tests. Here are the notes it had for me when I asked about major differences between the two implementations:
1. Provider & quantization — The originals used new_quant_index (inmem provider with a
trained PQ table). The new tests use test_provider::Provider::grid() — no quantization at
all. Since these tests are about multihop traversal behavior, not quantization accuracy, this
shouldn't matter, but it does mean the new tests exercise a simpler code path through the
accessor.

2. Start point filtering — This is the biggest behavioral difference. The inmem provider's
post-processor includes FilterStartPoints, which strips the start point from results. The
test provider does not filter start points. This forced a change in
reject_all_returns_zero_results: the original asserted result_count == 0, the new version
asserts zero non-start-point results. The other three tests weren't affected.

3. Async → sync — Originals were #[tokio::test] async fn. The new tests are #[test] fn using
current_thread_runtime().block_on(), matching the existing pattern in
diskann/src/graph/test/cases/.

4. Grid setup — Originals manually built vectors, adjacency lists, trained PQ, and called
populate_data/populate_graph. The new tests use Provider::grid() which does everything in one
call — less surface area, but also means the graph topology is generated differently (by
synthetic::Grid rather than the utils::genererate_3d_grid_adj_list helper).

5. Filter types — Moved as-is, no logic changes.

The start point difference (#2) is the one most worth flagging to your reviewer — it's a
genuine behavioral gap, not just a mechanical port. 

@JordanMaples JordanMaples force-pushed the jordanmaples/migrate_unittests branch from 2a56fc4 to 5a59f22 Compare April 8, 2026 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant