Skip to content

Comments

test: add public e2e harness with S3/LocalStack and drop test-helpers gating#534

Open
belveryin wants to merge 13 commits intodevfrom
e2e-coverage-upgrade
Open

test: add public e2e harness with S3/LocalStack and drop test-helpers gating#534
belveryin wants to merge 13 commits intodevfrom
e2e-coverage-upgrade

Conversation

@belveryin
Copy link
Collaborator

@belveryin belveryin commented Dec 10, 2025

  • Public API integration tests: Public-only scenarios live in tests/ (read smoke, WAL durability/rotation, time-travel). They use only DbBuilder/DB APIs; S3 runs remain opt-in via TONBO_S3_*.
  • Internal integration coverage: Broader, DbInner-driven tests moved under src/db/tests/ (compaction GC/loop, WAL policy/rotation, durability variants, scan plan, conflict, public API with S3/local harnesses, wasm-compat). These use crate-private helpers and cfg(test) knobs.
  • Harness & support: src/db/tests/backend.rs and src/test_support.rs provide WAL/FS/S3 harnesses and test helpers; tests/s3_localstack_env.sh remains for LocalStack setup.
  • CI/pre-commit: CI S3 jobs now run cargo test --package tonbo --lib public_api:: -- --nocapture; pre-commit runs fmt/clippy/tests and conditionally starts LocalStack, falling back to local-only if Docker isn’t available.
  • Path cleanup: Removed duplicate src/db/tests/integration namespace and collapsed tests/e2e/* into tests/*. Cargo.toml updated to point at the new test paths. No public API surface changes.

@belveryin belveryin requested a review from ethe December 10, 2025 13:27
Copy link
Member

@ethe ethe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Tests won't compile or run - compaction_gc.rs, conflict.rs, scan_plan.rs, time_travel.rs, wal_policy.rs, wal_rotation.rs, wasm_compat.rs are dead code.
  • workspace_temp_dir(), extract_rows(), wal_cfg_with_backend() copy-pasted across 7+ files
  • #[path = "common.rs"] is anti-pattern, it does align with the best practice provided by rust official.
  • both src/db/tests/backend.rs and src/tests_internal/backend.rs handle S3 setup

let descriptor = SsTableDescriptor::new(SsTableId::new(pass as u64 + 1), 0);
if let Err(err) = flush_immutables(&db, Arc::clone(&sst_cfg), descriptor).await {
eprintln!("flush skipped: {err}");
return Ok(());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Silent test skips hide failures

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.

2 participants