Test suites can pass vacuously: env-gated tests silently skip in CI (sibling of #389)

[aacruzgon]

Summary

Sibling of #389. While investigating the Docker outage there, an audit of the test suites and workflows found several other places where CI reports green while tests silently skip — the suite passes whether or not the gated tests ran. #389 covered the email-channel instance; this issue tracks the rest.

Rust's libtest has no runtime "skip" outcome, so availability guards must choose between passing and failing when a dependency is missing — and everywhere below they chose "pass," which converts missing infrastructure or unset variables into green checkmarks.

Confirmed gaps — green in CI, tests never run anywhere

Suite	Gate	Status
crates/persistence/tests/s3_es_tests.rs (S3+ES composite)	RUN_MINIO_S3_ES_TESTS=1	Never set in any workflow — the entire suite has never run in CI
8 tests in crates/persistence/tests/mongodb_tests.rs (count_by_types, count_all_types, count_by_day, count_deltas_by_bucket, activity_histogram, count_by_tenant, tenant_registry_crud, purge_tenant_data)	HFS_TEST_MONGODB_URL	Never set in any workflow. Main CI runs cargo test --workspace --all-features, so these compile and pass vacuously while their sibling tests in the same file run against a testcontainer
crates/persistence/tests/s3_tests.rs (real AWS S3)	RUN_AWS_S3_TESTS=1 + HFS_S3_TEST_BUCKET_TENANT_A/B	Never set — presumably intentional (needs live AWS credentials), but undocumented, so it reads as coverage that doesn't exist
crates/subscriptions/tests/email_integration.rs	docker_available() guard	Tracked in #389

Silent but covered elsewhere (acceptable, worth documenting)

• crates/auth/tests/jwks_cluster_redis.rs — skips unless RUN_REDIS_CLUSTER_TESTS=1; silent in main CI, but redis-cluster-tests.yml sets it and triggers on auth-path changes + nightly + dispatch.

The model pattern already exists in-repo

ci.yml handles minio_s3_tests.rs correctly: it sets RUN_MINIO_S3_TESTS=1 and has an anti-vacuous contract check (ci.yml:~109) — the comment reads "This step must not be able to pass vacuously" and the step asserts the expected number of tests actually ran, so a renamed test or typo'd variable turns the step red. This pattern just wasn't applied to the other gated suites.

Lower-risk observations

• The persistence capability harness (crates/persistence/tests/common/harness.rs, TestDecision::Skip) passes on skip. It's deterministic (capability-matrix-driven, not infra-dependent), but a wrongly-declared capability silently drops entire suites — relevant given fix(persistence): correct tenancy capability advertisements (#369) #379 is about incorrect capability advertisements.
• crates/serde/tests/test_examples.rs permanently skips known-problem resources (printed, but invisible in results).
• Not problems: audit-events.yml's exit 0s are readiness-wait loops; the || trues are cleanup; subscriptions-smoke.yml's MAILPIT_HTTP_URL gate is self-satisfied (the workflow starts mailpit itself).

Proposed remediation

1. Extend the MinIO anti-vacuous contract check to every env-gated suite that CI intends to run.
2. Set RUN_MINIO_S3_ES_TESTS=1 in CI (the suite uses MinIO via testcontainers, same infra as the covered MinIO suite).
3. Either point the 8 gated MongoDB tests at the same testcontainer their sibling tests use, or set HFS_TEST_MONGODB_URL in CI.
4. Wire s3_tests.rs into a scheduled workflow rather than leaving it manual-only. This is low-friction with the repo's existing AWS setup: the workflows already authenticate via OIDC federation (id-token: write + aws-actions/configure-aws-credentials@v6 with role-to-assume: ${{ secrets.AWS_ROLE_ARN }} — see bulk-export-smoke.yml, bulk-submit-smoke.yml, audit-events.yml), so no long-lived AWS secrets need to be stored or rotated; short-lived credentials are minted per run. The scheduled job would need: id-token: write, the configure-aws-credentials step (it exports the standard AWS_* env vars the SDK reads), RUN_AWS_S3_TESTS=1, and HFS_S3_TEST_BUCKET_TENANT_A/B values. AWS-side prerequisites: the IAM role's trust policy must allow that workflow to assume it, and the role must have access to the test buckets. Until that lands, document the suite as manual-only so its absence from CI is at least visible.
5. Adopt a convention for future guards: skip locally, fail when CI is set (as proposed in subscriptions-channels.yml fails on self-hosted Linux runners: no docker.sock (needs DOCKER_HOST env like smoke workflow) #389 for the email guard).

[aacruzgon]

Un-skipping the S3+ES suite (PR #391) revealed it was born broken — it had never passed anywhere, only appeared green via the silent skip. Two findings:

1. The test harness built its composite with the default SyncMode::Asynchronous but never called start_sync_workers() — with an S3 primary (no durable outbox), every secondary sync failed with "async worker was never started", and the composite swallows sync errors (warn! only). Nothing was ever indexed into ES. Fixed in ci+deps+test(persistence): run the S3+ES composite suite on every agent, fix its latent defects, and bump past the crc-fast x86_64 panic #391 by making the harness use SyncMode::Synchronous.

2. Possible product-level footgun for triage: the same silent-swallow behavior means a production composite configured with async sync whose workers were never started (or died) fails every secondary sync and only warns — silently losing the whole search index while writes report success. Worth considering whether repeated total sync failure should escalate beyond warn! (health status does track it, but nothing surfaces it loudly).

Also for the record: earlier revisions of #391 looked green because Test Rust landed on the Windows agent where the suite skipped — the ok. 10 passed ... finished in 0.01s signature in those logs is the exact vacuous-pass mode this issue describes, observed live while trying to fix it.