Skip to content

Conversation

@wjones127
Copy link
Contributor

Summary

  • Move compute-heavy vector index tests from unit test modules to integration tests
  • Tests run with opt-level=1 in the query-integration-tests CI job for better performance
  • Prevents CI timeouts caused by debug build slowness (6.5x slower than optimized)

Tests migrated:

  • test_build_ivf_pq, test_build_ivf_pq_v3, test_build_ivf_pq_4bit
  • test_create_ivf_hnsw_pq, test_create_ivf_hnsw_pq_4bit
  • test_optimize_with_empty_partition, test_remap_join_on_second_delta
  • test_create_index_with_many_invalid_vectors
  • test_build_pq_model_l2, test_build_pq_model_cosine

Test plan

  • cargo check -p lance --features slow_tests --tests passes
  • cargo clippy -p lance --features slow_tests --tests passes
  • CI passes

🤖 Generated with Claude Code

Move compute-heavy vector index tests from unit test modules to
integration tests where they run with opt-level=1 for better
performance. This prevents CI timeouts caused by debug build slowness.

Tests migrated:
- test_build_ivf_pq, test_build_ivf_pq_v3, test_build_ivf_pq_4bit
- test_create_ivf_hnsw_pq, test_create_ivf_hnsw_pq_4bit
- test_optimize_with_empty_partition, test_remap_join_on_second_delta
- test_create_index_with_many_invalid_vectors
- test_build_pq_model_l2, test_build_pq_model_cosine

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Contributor

PR Review Summary

This is a well-organized refactoring that moves compute-heavy vector index tests to integration tests. The change is straightforward and the motivation (CI timeout prevention) is sound.

Minor Observations

P1: Possible test coverage gap in moved tests

The original tests in v2.rs called multiple test helpers like test_remap, test_distance_range, test_index_multivec, and test_delete_all_rows in addition to the basic index creation. The new integration tests (ivf_pq.rs, ivf_hnsw.rs) only call test_recall. While some of these may be covered elsewhere, it's worth verifying that remap/delete functionality remains tested for these index types.

Note: test_spfresh_join_split was correctly kept in unit tests since it requires private API access - good decision documented in the PR.

Overall

The migration is clean, the shared utilities in utils.rs are well-factored, and the feature flag gating via slow_tests is appropriate. No bugs or security concerns identified.

@codecov
Copy link

codecov bot commented Jan 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

Move test_create_index_nulls (10 parameterized cases) and
test_initialize_vector_index_empty_dataset to the integration
test module to further reduce unit test runtime.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
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