test: add cross-surface integration suite and CI workflow#106
test: add cross-surface integration suite and CI workflow#106davidkelley wants to merge 1 commit intomainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 549557ff8e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .arg("build") | ||
| .arg("-p") | ||
| .arg("searchlite-cli") |
There was a problem hiding this comment.
The integration workflow runs cargo test -p integration --all-features (.github/workflows/integration-suite.yml), but this helper compiles searchlite-cli without propagating those features, so the CLI/HTTP process used by tests can silently run a different capability set than the test crate. cargo build --help explicitly documents --all-features as “Activate all available features”; without it here, feature-gated behavior (e.g., vectors/gpu/zstd paths) is not actually exercised in cross-surface runs and can regress undetected.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new integration workspace crate plus a dedicated CI workflow to run cross-surface integration tests that exercise Searchlite consistently across core (Rust API), HTTP, and CLI, seeded from examples/.
Changes:
- Introduce an
integration/crate with fixture loading, matrix generation, output normalization, and surface harness abstractions. - Add broad integration coverage: lifecycle matrix, feature-cross matrix, expected-output parity, adversarial cases, and surface-specific contract tests.
- Wire up developer/CI entrypoints via a
Justfiletarget set and a new GitHub Actions workflow (quick on PRs, full nightly/manual).
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
integration/tests/surface_smoke.rs |
Basic smoke tests for core/CLI/HTTP harnesses. |
integration/tests/lifecycle_matrix.rs |
Lifecycle coverage across datasets/surfaces/execution strategies with capability-aware assertions. |
integration/tests/harness_smoke.rs |
Compile-time smoke coverage for the SurfaceHarness trait integration. |
integration/tests/fixtures_loading.rs |
Validates fixture parsing from examples/ and derived mutation payloads. |
integration/tests/feature_cross_matrix.rs |
Validates matrix generation properties (size, uniqueness, sharding determinism). |
integration/tests/expected_outputs.rs |
Core vs HTTP normalized output parity over example queries. |
integration/tests/contracts_http.rs |
HTTP error/status contract checks and structured error shape assertions. |
integration/tests/contracts_core.rs |
Core surface contract checks for stats/inspect and mget behavior. |
integration/tests/contracts_cli.rs |
CLI contract checks for failure behavior and JSON output. |
integration/tests/common/mod.rs |
Shared helpers for locating/building the searchlite/searchlite-cli binary and NDJSON formatting. |
integration/tests/adversarial_matrix.rs |
Negative/adversarial cross-surface coverage (invalid schema, malformed NDJSON, pagination conflicts). |
integration/src/surfaces/mod.rs |
Defines SurfaceHarness, SurfaceKind, capability map, and unsupported-operation marker. |
integration/src/surfaces/http.rs |
HTTP harness that spawns the server and drives endpoints via reqwest. |
integration/src/surfaces/core.rs |
Core harness adapter over searchlite_core Index/Reader/Writer APIs. |
integration/src/surfaces/cli.rs |
CLI harness adapter executing the binary with scratch files for schema/docs/requests. |
integration/src/scenario.rs |
Basic scenario case/outcome structs used by integration harnesses. |
integration/src/normalization.rs |
Normalizes search results for cross-surface parity comparisons. |
integration/src/matrix.rs |
Matrix mode/sharding and case generation for feature hardening. |
integration/src/lib.rs |
Exposes integration crate modules. |
integration/src/fixtures.rs |
Loads and normalizes schemas/docs/queries from examples/ and derives mutation fixtures. |
integration/src/assertions.rs |
Parity assertions built on normalized search output. |
integration/Cargo.toml |
Declares the new integration crate, features, and dependencies. |
docs/feature-hardening/cross-surface-integration-suite/matrix.md |
Feature-hardening matrix documenting scope, invariants, and test references. |
Justfile |
Adds test-integration-quick / test-integration-full entrypoints. |
Cargo.toml |
Adds integration to workspace members. |
Cargo.lock |
Locks new dependencies pulled in by the integration crate. |
.github/workflows/integration-suite.yml |
Adds CI workflow to run integration suite (quick on PR, full nightly/manual). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| use serde_json::Value; | ||
|
|
||
| pub fn searchlite_bin() -> PathBuf { | ||
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite") { | ||
| return PathBuf::from(path); | ||
| } | ||
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite-cli") { | ||
| return PathBuf::from(path); | ||
| } | ||
|
|
||
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | ||
| .parent() | ||
| .expect("workspace root") | ||
| .to_path_buf(); | ||
|
|
||
| let candidates = [ | ||
| workspace_root | ||
| .join("target") | ||
| .join("debug") | ||
| .join(if cfg!(windows) { | ||
| "searchlite.exe" | ||
| } else { | ||
| "searchlite" | ||
| }), | ||
| workspace_root | ||
| .join("target") | ||
| .join("debug") | ||
| .join(if cfg!(windows) { | ||
| "searchlite-cli.exe" | ||
| } else { | ||
| "searchlite-cli" | ||
| }), | ||
| ]; | ||
| for candidate in candidates { | ||
| if candidate.exists() { | ||
| return candidate; | ||
| } | ||
| } | ||
|
|
||
| let status = Command::new("cargo") | ||
| .arg("build") | ||
| .arg("-p") | ||
| .arg("searchlite-cli") | ||
| .current_dir(&workspace_root) | ||
| .status() | ||
| .expect("build searchlite binary"); | ||
| assert!(status.success(), "building searchlite-cli failed"); | ||
|
|
||
| workspace_root | ||
| .join("target") | ||
| .join("debug") | ||
| .join(if cfg!(windows) { | ||
| "searchlite-cli.exe" | ||
| } else { | ||
| "searchlite-cli" | ||
| }) |
There was a problem hiding this comment.
searchlite_bin() falls back to spawning cargo build from inside the test process. Since tests run in parallel, multiple tests can concurrently hit this path and race on the target-dir/Cargo locks, causing flaky/hung CI. Also the build is invoked without propagating the current feature set (--all-features / selected features), so the CLI/HTTP binary may not match the feature matrix being tested.
Consider memoizing the resolved path (e.g., OnceLock<PathBuf>) and building once up-front (or, preferably, build the binary in the workflow/Justfile before running tests and make searchlite_bin() purely a locator).
| use serde_json::Value; | |
| pub fn searchlite_bin() -> PathBuf { | |
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite") { | |
| return PathBuf::from(path); | |
| } | |
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite-cli") { | |
| return PathBuf::from(path); | |
| } | |
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | |
| .parent() | |
| .expect("workspace root") | |
| .to_path_buf(); | |
| let candidates = [ | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite.exe" | |
| } else { | |
| "searchlite" | |
| }), | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite-cli.exe" | |
| } else { | |
| "searchlite-cli" | |
| }), | |
| ]; | |
| for candidate in candidates { | |
| if candidate.exists() { | |
| return candidate; | |
| } | |
| } | |
| let status = Command::new("cargo") | |
| .arg("build") | |
| .arg("-p") | |
| .arg("searchlite-cli") | |
| .current_dir(&workspace_root) | |
| .status() | |
| .expect("build searchlite binary"); | |
| assert!(status.success(), "building searchlite-cli failed"); | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite-cli.exe" | |
| } else { | |
| "searchlite-cli" | |
| }) | |
| use std::sync::OnceLock; | |
| use serde_json::Value; | |
| static SEARCHLITE_BIN: OnceLock<PathBuf> = OnceLock::new(); | |
| pub fn searchlite_bin() -> PathBuf { | |
| SEARCHLITE_BIN | |
| .get_or_init(|| { | |
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite") { | |
| return PathBuf::from(path); | |
| } | |
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite-cli") { | |
| return PathBuf::from(path); | |
| } | |
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | |
| .parent() | |
| .expect("workspace root") | |
| .to_path_buf(); | |
| let candidates = [ | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite.exe" | |
| } else { | |
| "searchlite" | |
| }), | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite-cli.exe" | |
| } else { | |
| "searchlite-cli" | |
| }), | |
| ]; | |
| for candidate in candidates { | |
| if candidate.exists() { | |
| return candidate; | |
| } | |
| } | |
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | |
| .parent() | |
| .expect("workspace root") | |
| .to_path_buf(); | |
| let status = Command::new("cargo") | |
| .arg("build") | |
| .arg("--all-features") | |
| .arg("-p") | |
| .arg("searchlite-cli") | |
| .current_dir(&workspace_root) | |
| .status() | |
| .expect("build searchlite binary"); | |
| assert!(status.success(), "building searchlite-cli failed"); | |
| workspace_root | |
| .join("target") | |
| .join("debug") | |
| .join(if cfg!(windows) { | |
| "searchlite-cli.exe" | |
| } else { | |
| "searchlite-cli" | |
| }) | |
| }) | |
| .clone() |
| fn searchlite_bin() -> PathBuf { | ||
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite") { | ||
| return PathBuf::from(path); | ||
| } | ||
| if let Ok(path) = std::env::var("CARGO_BIN_EXE_searchlite-cli") { | ||
| return PathBuf::from(path); | ||
| } | ||
|
|
||
| let workspace_root = PathBuf::from(env!("CARGO_MANIFEST_DIR")) | ||
| .parent() | ||
| .expect("workspace root") | ||
| .to_path_buf(); | ||
| let candidates = [ |
There was a problem hiding this comment.
This file re-implements searchlite_bin() instead of using integration/tests/common::searchlite_bin(), and it also triggers an internal cargo build fallback. That duplication increases maintenance cost and, more importantly, the build fallback can race when the three tests in this binary run in parallel.
Prefer reusing the shared helper and ensuring the binary build happens once (memoized) or before tests start.
| let mut hasher = std::collections::hash_map::DefaultHasher::new(); | ||
| input.hash(&mut hasher); | ||
| hasher.finish() as usize |
There was a problem hiding this comment.
stable_hash() uses std::collections::hash_map::DefaultHasher, which is not a stability guarantee across Rust versions/platforms. Because this hash drives quick-mode thinning and sharding, upgrading the toolchain can silently reshuffle the case set and break reproducibility.
Use a hash algorithm with a documented stable output (e.g., a fixed-seed FNV-1a/xxhash implementation) so the matrix selection stays deterministic across environments.
| let mut hasher = std::collections::hash_map::DefaultHasher::new(); | |
| input.hash(&mut hasher); | |
| hasher.finish() as usize | |
| // Deterministic, platform- and version-stable hash (FNV-1a 64-bit). | |
| const FNV_OFFSET_BASIS: u64 = 0xcbf29ce484222325; | |
| const FNV_PRIME: u64 = 0x100000001b3; | |
| let mut hash: u64 = FNV_OFFSET_BASIS; | |
| for byte in input.as_bytes() { | |
| hash ^= *byte as u64; | |
| hash = hash.wrapping_mul(FNV_PRIME); | |
| } | |
| hash as usize |
| .stdout(Stdio::null()) | ||
| .stderr(Stdio::null()) |
There was a problem hiding this comment.
The harness discards the HTTP server process stdout/stderr (Stdio::null()), which makes integration failures hard to diagnose (especially when wait_for_health times out or the process exits early).
Consider capturing stderr (e.g., Stdio::piped()) and including recent output in error paths, or inherit it under RUST_LOG/an env flag so CI logs contain actionable diagnostics.
| .stdout(Stdio::null()) | |
| .stderr(Stdio::null()) | |
| .stdout(Stdio::inherit()) | |
| .stderr(Stdio::inherit()) |
| serde = { workspace = true, features = ["derive"] } | ||
| serde_json = { workspace = true } | ||
| tempfile = { workspace = true } | ||
| tokio = { version = "1.37.0", features = ["macros", "rt-multi-thread", "time"] } |
There was a problem hiding this comment.
tokio is declared as a regular dependency but does not appear to be used anywhere in the integration crate (src or tests). Keeping unused dependencies increases compile time and dependency surface for the workspace.
If not needed yet, remove it (or move it to dev-dependencies when/if async tests are introduced).
| tokio = { version = "1.37.0", features = ["macros", "rt-multi-thread", "time"] } |
Summary
integrationworkspace crate with cross-surface harnesses for core, HTTP, and CLIexamples/Test Plan