Skip to content

test(clients/python): commit before force_tick in 2 tests#139

Merged
NikolayS merged 1 commit intomainfrom
fix/clients-python-test-failures-r2
Apr 30, 2026
Merged

test(clients/python): commit before force_tick in 2 tests#139
NikolayS merged 1 commit intomainfrom
fix/clients-python-test-failures-r2

Conversation

@NikolayS
Copy link
Copy Markdown
Owner

Summary

Two Python tests regressed on the live-pg test job because they call pgque.force_tick + pgque.ticker in the same transaction as the preceding client.send / client.send_batch, without committing in between. pgque.ticker() captures a snapshot of currently-committed xacts, so an event inserted in the same still-open xact is not visible to the tick. The subsequent pgque.receive therefore returns an empty batch, and the dependent assertions fail.

The other tests in the same files already commit between send and tick; this brings the two regressed tests in line with that pattern.

Failing tests fixed:

  • tests/test_consumer.py::test_consumer_warns_and_acks_unhandled_event_type
  • tests/test_send.py::test_send_batch_none_payload_produces_json_null

No driver code is changed; both failures were test-only bugs about pgque snapshot semantics.

Test plan

  • pytest -v clients/python/tests/ against a live PG with pgque.sql from main -> 41/41 pass
  • CI Python client tests job goes green

Generated by Claude Code

The two affected tests insert a message via client.send / client.send_batch
and immediately call pgque.force_tick + pgque.ticker without committing in
between. Because the tick captures a snapshot of currently-committed xacts,
the just-inserted event is not visible to the tick, so pgque.receive returns
an empty batch and the assertions fail.

The other tests in the same file already commit between send and tick; this
brings the two regressed tests in line with that pattern.
Copy link
Copy Markdown
Owner Author

REV review — PR #139

Verdict: PASS (non-blocking nit)

1. Security

N/A — test-only.

2. Bugs / root cause

Fix is correct and addresses root cause. pgque.ticker() snapshots committed xacts; an event inserted in the producer's still-open xact is invisible to the tick, so the subsequent receive returns [] and the assertions (len(...) == 1, len(warning_lines) >= 1, retry_queue count) fail. Adding conn.commit() after client.send / client.send_batch and before pgque.force_tick matches the resolution pattern explicitly established by #129. No risk of "passing for the wrong reason": both tests still drive the producer through the public client.send* API on the same conn (pyscopg in non-autocommit mode), so the commit boundary is real, not a driver-quirk hide. The two-arg vs three-arg send() selection and the send_batch([None]) JSON-null coercion are still exercised before the commit.

3. Test analysis

Both test bodies still assert what their names claim:

  • test_consumer_warns_and_acks_unhandled_event_type — still validates (a) zero rows in pgque.retry_queue (ack, not nack) and (b) a WARNING log mentioning the event type and msg id.
  • test_send_batch_none_payload_produces_json_null — still validates that send_batch([None]) round-trips as JSON null (not SQL NULL); the commit is on the producer side and does not interact with payload coercion.
    No tests are silently passing because of the fix. The added commit is not "swallowing" a separate driver bug — send_batch's JSON-null behavior is encoded in the SQL/cast layer and would fail identically with or without the commit if it regressed.

4. Guidelines

  • Conventional commits: test(clients/python): ... — compliant.
  • Anti-leak: clean. No person names, GitLab refs, internal infra, Round/WI markers. Body references PRs by number only.
  • Diff is +2/-0 across two files — minimal, surgical.

5. Docs

N/A.

6. Architecture

CLAUDE.md rule (drivers mirror SQL API 1:1): unchanged — no public client API touched, no composite "send-then-tick" client method invented. Consistent with #129.

Non-blocking observation: this is the third Python PR in the sprint to land on the same "commit between send and force_tick" footgun (#129 established the pattern; this PR fixes two stragglers that were merged before that pattern was widespread). Recommend a small private test helper, e.g.

# clients/python/tests/_helpers.py
def commit_then_advance(conn, queue: str) -> None:
    """Commit producer xact, then force a tick so ticker() sees the events."""
    conn.commit()
    conn.execute("select pgque.force_tick(%s)", (queue,))
    conn.execute("select pgque.ticker()")
    conn.commit()

Then call commit_then_advance(conn, queue) at every send/receive boundary. This collapses the 4-line ritual to one line, makes the snapshot-isolation invariant the helper's responsibility, and stops the bug from being reintroduced one test at a time. Not blocking this PR — suggested as a follow-up.

LGTM to merge once Python client tests goes green.


Generated by Claude Code

@NikolayS NikolayS merged commit f2d3559 into main Apr 30, 2026
8 checks passed
@NikolayS NikolayS deleted the fix/clients-python-test-failures-r2 branch April 30, 2026 23:13
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