Use VirtualStorageProvider::new_overlay(test_data_root()) in tests - #726
Conversation
Co-authored-by: arrayka <1551741+arrayka@users.noreply.github.com>
Co-authored-by: arrayka <1551741+arrayka@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR standardizes test data resolution across the diskann-providers crate by replacing hardcoded workspace root path patterns with the centralized test_data_root() helper function from diskann-utils. The changes are part of a broader effort (following PR #700) to ensure tests use the virtual filesystem overlay pattern consistently and avoid direct filesystem writes during testing.
Changes:
- Replaced
std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).parent().unwrap()patterns withtest_data_root()across 17 test functions - Updated test file path constants and variables to remove the
/test_data/prefix (sincetest_data_root()already points to the test_data directory) - Added diskann-utils with "testing" feature to dev-dependencies to access the
test_data_root()function
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| diskann-providers/src/utils/normalizing_util.rs | Updated 1 test function to use test_data_root() and removed /test_data/ prefix from paths |
| diskann-providers/src/utils/kmeans.rs | Updated 1 test function to use test_data_root(), removed unused PathBuf import, and fixed path prefix |
| diskann-providers/src/storage/pq_storage.rs | Updated 4 test functions and 3 const path declarations to use test_data_root() with corrected path prefixes |
| diskann-providers/src/storage/index_storage.rs | Updated 1 test function to use test_data_root() and corrected file path |
| diskann-providers/src/model/pq/pq_construction.rs | Updated 4 test functions and const path declarations to use test_data_root() with corrected prefixes |
| diskann-providers/src/model/pq/fixed_chunk_pq_table.rs | Updated 3 test functions to use test_data_root() and removed /test_data/ prefix from paths |
| diskann-providers/src/index/diskann_async.rs | Updated 3 test functions and 1 const path declaration to use test_data_root() with corrected paths |
| diskann-providers/Cargo.toml | Added diskann-utils with "testing" feature to dev-dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #726 +/- ##
==========================================
- Coverage 89.01% 89.00% -0.01%
==========================================
Files 428 428
Lines 78294 78234 -60
==========================================
- Hits 69692 69632 -60
Misses 8602 8602
🚀 New features to boost your workflow:
|
Aditya Krishnan (arkrishn94)
left a comment
There was a problem hiding this comment.
LGTM
## What's Changed ### API Breaking Changes * Remove the `experimental_avx512` feature. by @hildebrandmw in #732 * Use VirtualStorageProvider::new_overlay(test_data_root()) in tests by @Copilot in #726 * save and load max_record_size and leaf_page_size for bftrees by @backurs in #724 * [multi-vector] Verify `Standard` won't overflow in its constructor. by @hildebrandmw in #757 * VirtualStorageProvider: Make new() private, add new_physical by @Copilot in #764 * [minmax] Refactor full query by @arkrishn94 in #770 * Bump diskann-quantization to edition 2024. by @hildebrandmw in #772 ### Additions * [multi-vector] Enable cloning of `Mat` and friends. by @hildebrandmw in #759 * adding bftreepaths in mod.rs by @backurs in #775 * [quantization] Add `as_raw_ptr`. by @hildebrandmw in #774 ### Bug Fixes * Fix `diskann` compilation without default-features and add CI tests. by @hildebrandmw in #722 ### Docs and Comments * Updating the benchmark README to use diskann-benchmark by @bryantower in #709 * Fix doc comment: Windows line endings are \r\n not \n\r by @Copilot in #717 * Fix spelling errors in streaming API documentation by @Copilot in #715 * Add performance diagnostic to `diskann-benchmark` by @hildebrandmw in #744 * Add agents.md onboarding guide for coding agents by @Copilot in #765 * [doc] Fix lots of little typos in `diskann-wide` by @hildebrandmw in #771 ### Performance * [diskann-wide] Optimize `load_simd_first` for 8-bit and 16-bit element types. by @hildebrandmw in #747 ### Dependencies * Bump bytes from 1.11.0 to 1.11.1 by @dependabot[bot] in #723 * [diskann] Add note on the selection of `PruneKind` in `graph::config::Builder`. by @hildebrandmw in #734 * [diskann-providers] Remove the LRU dependency and make `vfs` and `serde_json` optional. by @hildebrandmw in #733 ### Infrastructure * Add initial QEMU tests for `diskann-wide`. by @hildebrandmw in #719 * [CI] Skip coverage for Dependabot. by @hildebrandmw in #725 * Add miri test coverage to CI workflow by @Copilot in #729 * [CI] Add minimal ARM checks by @hildebrandmw in #745 * Enable CodeQL security analysis by @Copilot in #754 ## New Contributors * @backurs made their first contribution in #724 * @arkrishn94 made their first contribution in #770 **Full Changelog**: 0.45.0...0.46.0
Test files were using hardcoded workspace root paths (
env!("CARGO_MANIFEST_DIR").parent().unwrap()) instead of the centralizedtest_data_root()helper from diskann-utils.Changes
test_data_root()across 17 test functions in 7 files/test_data/prefix from test file paths sincetest_data_root()already resolves to the test_data directorytestingfeature for diskann-utils in dev-dependenciesBefore
After
Files affected:
diskann-providers/src/index/diskann_async.rsdiskann-providers/src/storage/{pq_storage.rs, index_storage.rs}diskann-providers/src/utils/{normalizing_util.rs, kmeans.rs}diskann-providers/src/model/pq/{fixed_chunk_pq_table.rs, pq_construction.rs}Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.