Skip to content

Test gaps at 3fa3533: surviving mutants and test names that claim more than they check #104

Closed

Description

@vheathen

Follow-up to #81.

After #92, #93 and #94, 22 of the 24 mutants I ran on 3fa3533 for the items of #81 fail a test. The mutants below still pass, and some tests check less than their names say. "The lock tests" are the 87 tests in test/advisory_locks_test.exs, test/advisory_locks_protocol_test.exs, test/storage/subscription_lock_test.exs and test/subscriptions/subscription_locking_test.exs. Each item has its details below.

Details

Each mutant was applied to 3fa3533 and the named test files were run against it; every test passed unless the item says otherwise.

  • AdvisoryLocks (fix(subscriptions): stop a stale verification retry from polling alongside the pending one #67): with state = cancel_verify_retry(state) replaced by state = state in the lock pool :DOWN handler, the lock tests passed (87), and so did 377 tests in advisory_locks_test.exs, advisory_locks_protocol_test.exs, test/storage and test/subscriptions.

  • Tasks.Migrations (fix(migrations): keep the fork's migrations out of upstream's schema versions #89): FROM "public".fork_schema_migrations in place of FROM "#{schema}".fork_schema_migrations in query_fork_schema_migrations/2 passed all 22 tests of list_event_store_migrations_test.exs, migrate_event_store_test.exs and migrated_event_store_test.exs under MIX_ENV=migration --include migration. fix(migrations): keep the fork's migrations out of upstream's schema versions #89 removed lock_generation?/2, so I made new mutants that drop or hardcode the schema in the queries of Tasks.Migrate and Tasks.Migrations; of five, this is the only one no test catches. Leaving the schema out of the to_regclass check, or reading "public".schema_migrations in the same task, fails "event store in its own schema beside a migrated one lists the migrations of its own schema".

  • AdvisoryLocks and AdvisoryLocks.Protocol (fix(subscriptions): tell lock owners their session is gone when the call that hit it replies before the connection says so #87): each of these passed the lock tests in 3 of 3 runs:

    • without_retry({:disconnect_and_retry, error, state}) returning {:disconnect, error, state} directly, so no :query_disconnected marker is sent;
    • 0 -> state in place of 0 -> receive_session_loss(state, 0) in handle_pending_session_loss/1, so a :session_lost notice already in the mailbox is not handled before the next call;
    • handle_info({AdvisoryLocks, :query_disconnected}, state) returning {:noreply, state} without waiting for the notice;
    • @session_lost_wait set to :infinity. In no lock test does the notice fail to arrive, so nothing checks that the wait ends.
  • Listener (fix(subscriptions): make caught up subscriptions read events appended while the notification connection was not listening #82): each mutant passed 20 of 20 runs of subscription_listen_gap_test.exs (3 tests) and one run of test/notifications (8 tests, 2 doctests). The listen-gap tests do not restart the notification children, and the only test that sets :configure passes it as {module, function, args}.

  • SubscriptionFsm (fix(subscriptions): stop retrying a failed checkpoint write on every ack #91): with seeds 1, 2 and 3, each passed the 33 tests of subscription_fsm_test.exs, and with seed 1 the 318 tests of test/subscriptions and test/storage:

    • the :ok branch of write_checkpoint/1 without checkpoint_unwritten: false, so after a failed write and a good one the flag stays set and every later persist_checkpoint/1 writes again with nothing pending;
    • the same in the :ok branch of delete/1;
    • checkpoints_pending: -1 in place of 0 in the error branch of write_checkpoint/1, which moves the next write after a failure one ack later.
  • Storage.Lock (fix(subscriptions): stop a losing claim from writing the subscription row #90): without the {:error, _error} = reply -> reply clause in try_acquire_exclusive_transaction_lock/4, advisory_locks_test.exs, subscription_lock_test.exs and subscription_locking_test.exs passed (86 tests) with seeds 1, 2 and 3. To see what the clause does, I revoked EXECUTE on pg_try_advisory_xact_lock(int, int) from PUBLIC and ran the session mode connection as a role without it, so that exactly this statement returns {:error, %Postgrex.Error{}}. On the real code the claim returned the error and AdvisoryLocks stayed up. With the mutant the call exited and AdvisoryLocks terminated with {:case_clause, {:error, %Postgrex.Error{postgres: %{code: :insufficient_privilege}}}}: with_bounded_lock_wait/3 rescues only DBConnection.ConnectionError and Postgrex.Error, not a CaseClauseError.

  • "migrate an event store in its own schema should leave other event stores in the same database untouched" (as chore(test): catch migration regressions and make two test barriers reliable #94 reworked it): besides the migration history, table_structure/2 reads information_schema.columns for public, so an added column is caught (the Test gaps at a2ce825: surviving mutants and two unreliable test barriers #81 mutant that adds one to public.streams now fails it), but other objects are not. A subscription_lock_generation.sql that runs CREATE INDEX IF NOT EXISTS mutant_idx ON public.streams (stream_version) when current_schema() <> 'public' passed all 8 tests of migrate_event_store_test.exs in 3 of 3 runs. With RAISE EXCEPTION under the same condition, exactly that test fails, so the statement does run during it.

  • "ack/3 after a checkpoint write failed waits for another threshold of acks before writing again" (fix(subscriptions): stop retrying a failed checkpoint write on every ack #91) uses threshold 5 and acks 1 to 8, then asserts one write attempt and no second one; the next threshold, at ack 10, is never reached, which is why the checkpoints_pending: -1 mutant above passes. "... writes the latest acknowledged position when the retry fires" swaps the schema and calls SubscriptionFsm.checkpoint/1 directly; no timer and no :checkpoint message are involved.

  • "checkpoint/1 writes the position when the retry timer fires after the checkpoint interval" (chore(test): catch subscription regressions the suite let through #93) does assert_receive :checkpoint, 1_000 and then calls SubscriptionFsm.checkpoint/1 itself, so the message never goes through Subscription.handle_info(:checkpoint, ...). "checkpoint/1 retries a failed write once the checkpoint interval has passed" and "... after the retry interval when it writes on every ack" (fix(subscriptions): keep a checkpoint that failed to write so the next one retries it #71) assert only a Process.read_timer/1 range.

  • "event store that recorded upstream's 1.5.0 lists the lock generation migration as pending" calls create_new_database/1, drops fork_schema_migrations and lock_generation, and inserts no 1.5.0 row. Since fix(migrations): keep the fork's migrations out of upstream's schema versions #89, Init records 1.3.2, so the test runs against a database with no 1.5.0 row at all; at a2ce825 Init recorded 1.5.0 and the name matched.

  • "closing a connection that cannot cancel its query gives up after a second even with an infinite timeout" (chore(test): catch lock regressions the lock tests let through #92) allows 5 s (Task.yield(closing, 5_000)), and passed with @cancel_timeout set to 2_000. That mutant still fails the lock tests in 3 of 3 runs, through the two "lock session ends while a release is queued and new connections hang" tests, which wait 1500 ms. "a claim gives up on a locked subscription row at the stricter session lock timeout" (chore(test): catch lock regressions the lock tests let through #92) sets lock_timeout to 100ms and asserts that the claim returns within 2 s; a claim that always waits 1.9 s and ignores the stricter session value passed it in 3 of 3 runs, and was caught by "should keep the lock held when a repeated claim cannot wait for a table lock" and "should keep every lock on the node while a table lock outlasts a claim". A claim with a finite :timeout whose wait is capped at 2 s (min(div(timeout, 2), 2_000)) passed all 87 lock tests in 3 of 3 runs.

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions