Skip to content

Eight Postgres integration tests are orphaned dead code, and hang when wired in #545

Description

@0w3n-d

What's wrong

crates/database/src/postgres/postgres_db_service_tests.rs is never declared
as a module. crates/database/src/postgres/mod.rs lists five modules and not
this one, so the file has never been compiled or run since the crate
reorganisation (0b3d3577 move crates to relay). Eight integration tests have
been silently absent for that whole time:

test_save_and_get_known_validators   test_late_payloads
test_save_large_batch                test_failed_payloads
test_get_bids                        test_gossiped_payload
test_get_delivered_payloads          test_load_functions_timing

cargo test -p helix-database --all-features runs exactly two tests, both
from postgres_db_u256_parsing.rs.

Two consequences, and the second is the reason this is a bug and not a
cleanup:

1. just test never needed Postgres. No test in the workspace opens a
database connection — the only file that would is this orphan. CLAUDE.md
says just test "requires a local Postgres (just local-postgres) for
helix-database's tests", which is stale and should be corrected.

2. The tests hang when wired in. Declaring #[cfg(test)] mod postgres_db_service_tests; needs two small fixes to compile — crate::database::postgres
should be crate::postgres, and ValidatorPreferences gained an api_key
field. After that all eight tests hang for over 60 seconds and never open a
connection to Postgres at all:

running 9 tests
test ...::test_load_functions_stress ... ignored
test ...::test_failed_payloads has been running for over 60 seconds
test ...::test_get_bids has been running for over 60 seconds
[... all 8 ...]

Verified with the database reachable (pg_isready OK on both 127.0.0.1:5432
and [::1]:5432) and pg_stat_activity showing no client backend from the
test process. So the tests block before connecting.

Suspected cause

All eight call run_setup().await, which awaits a shared
tokio::sync::OnceCell:

static SETUP: OnceCell<()> = OnceCell::const_new();
async fn run_setup() {
    SETUP.get_or_init(|| async { setup_test_conn().await.unwrap() }).await;

Each #[tokio::test] builds its own current-thread runtime, and the tests run
in parallel. One runtime wins the OnceCell permit and runs the initializer;
the other seven wait on a cell whose initializer lives on a runtime they do
not drive. A shared OnceCell across per-test runtimes is the likely
deadlock. -- --test-threads=1 would confirm it.

Repro

# in crates/database/src/postgres/mod.rs
echo '#[cfg(test)] mod postgres_db_service_tests;' >> crates/database/src/postgres/mod.rs
# fix the two compile errors, then
cargo test -p helix-database --all-features postgres_db_service_tests

Affected surface

crates/database/src/postgres/mod.rs,
crates/database/src/postgres/postgres_db_service_tests.rs, CLAUDE.md,
and justfile / .github/workflows/unit_test.yml for step 3.

Steps (each becomes one PR)

  • Step 1: Correct CLAUDE.md: just test needs no Postgres today. Independent of the rest and worth doing immediately so nobody else starts a container for nothing. (tests: n/a) (PR: )
  • Step 2: Wire the module in, fix the two compile errors, and fix the hang — most likely by replacing the shared OnceCell with per-test setup, or by forcing --test-threads=1 for this module. Confirm all eight pass against a real Postgres. (tests: the eight tests themselves, which is the point) (PR: )
  • Step 3: Keep just test dependency-free once they run: exclude the module with -- --skip postgres_db_service_tests, add a test-db recipe that runs exactly it, and run both in CI against the existing postgres service. Note that #[ignore] is not available as the mechanism — this repo already uses it for known-failing tests, so --ignored would pull those in. (tests: existing) (PR: )

Open questions

Are these eight tests worth reviving, or has their coverage been superseded?
They were written against an older schema, so Step 2 may turn up genuine
mismatches rather than just the deadlock. If the answer is "not worth it",
deleting the file is the honest alternative — but silently keeping dead test
code is not.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions