test: remove mock tests

[quettabit]

No description provided.

[greptile-apps]

Greptile Summary

This PR deletes tests/test_producer.py (335 lines, 14 tests) and tests/test_session.py (743 lines, 28 tests) with the stated intent of removing mock-based tests, presumably ahead of integration-test replacements. No new tests are added in this changeset.

• The deleted files covered Producer batching, linger-timer flushing, drain ordering, error propagation, AppendSession backpressure, S2S framing, ack-monotonicity validation, ack-timeout, frame-signal reset, and has_no_side_effects logic.
• Several removed tests (the _AppendPermits/_Semaphore concurrency tests, the ConnectError has_no_side_effects tests, and the frame-signal reset test) use no mocks and test pure or near-pure logic that integration tests cannot reliably cover.

Confidence Score: 3/5

Merging leaves the Producer, AppendSession backpressure, and several retry-logic paths with no test coverage at all until replacement tests arrive.

Over 1000 lines of tests are deleted with nothing added. A subset of the removed tests exercised pure logic (ConnectError chain-walk, _AppendPermits blocking, frame-signal reset) that no existing test file now covers, and integration tests are unlikely to reproduce those exact conditions. Until replacement tests land, regressions in these paths could go undetected.

Both deleted files should be reviewed against any forthcoming integration-test PR to confirm all removed scenarios, especially the concurrency and retry-logic edge cases, are re-covered.

Important Files Changed

Filename	Overview
tests/test_producer.py	Deleted: 335 lines covering 14 unit tests for Producer batching, linger timer, error propagation, drain ordering, match_seq_num auto-increment, and close flushing — no replacement tests added.
tests/test_session.py	Deleted: 743 lines covering 28 unit/integration-style tests for AppendSession, S2S framing, _AppendPermits/_Semaphore backpressure, ack-monotonicity enforcement, ack-timeout, frame-signal reset, ReadUnwrittenError, and has_no_side_effects — several of these tests use no mocking at all and have no remaining equivalent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[test_producer.py DELETED] -->|14 tests removed| B[Producer logic uncovered]
    B --> B1[Batching / linger timer]
    B --> B2[match_seq_num auto-increment]
    B --> B3[Drain FIFO ordering]
    B --> B4[Error propagation to tickets]

    C[test_session.py DELETED] -->|28 tests removed| D[AppendSession logic uncovered]
    D --> D1[Backpressure: _AppendPermits / _Semaphore]
    D --> D2[S2S framing encode/retry]
    D --> D3[Ack monotonicity enforcement]
    D --> D4[Ack timeout → ReadTimeoutError]
    D --> D5[Frame signal reset after resend]
    D --> D6[has_no_side_effects + ConnectError chain]

    style A fill:#f88,stroke:#c00
    style C fill:#f88,stroke:#c00

Loading

Comments Outside Diff (2)

1. tests/test_session.py

   Non-mock unit tests removed alongside mock tests

   The three test_connect_error_* tests at the bottom of this file use no mocking at all — they construct real ConnectError instances and assert on has_no_side_effects. Similarly, test_terminal_416_raises_read_unwritten exercises read_messages with a real byte stream, and test_frame_signal_reset_after_resend directly calls _run_attempt. None of these require a live service. Removing them means that has_no_side_effects, its ConnectionRefusedError chain-walk, and the _run_attempt frame-signal reset path are no longer exercised by any test in the suite. If the intent is to replace mock-based tests with integration tests, these lightweight unit tests for pure logic could be kept as-is, since integration tests are unlikely to cover the exact error-chain and frame-signal edge cases.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/test_session.py
   Line: 1066-1090
   
   Comment:
   **Non-mock unit tests removed alongside mock tests**
   
   The three `test_connect_error_*` tests at the bottom of this file use no mocking at all — they construct real `ConnectError` instances and assert on `has_no_side_effects`. Similarly, `test_terminal_416_raises_read_unwritten` exercises `read_messages` with a real byte stream, and `test_frame_signal_reset_after_resend` directly calls `_run_attempt`. None of these require a live service. Removing them means that `has_no_side_effects`, its `ConnectionRefusedError` chain-walk, and the `_run_attempt` frame-signal reset path are no longer exercised by any test in the suite. If the intent is to replace mock-based tests with integration tests, these lightweight unit tests for pure logic could be kept as-is, since integration tests are unlikely to cover the exact error-chain and frame-signal edge cases.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

2. tests/test_session.py

   Concurrency primitive tests have no integration test substitute

   _AppendPermits and _Semaphore are internal concurrency primitives whose blocking/unblocking behaviour under byte-count and batch-count limits was tested by six dedicated tests here. Integration tests running against a real service cannot reproduce these conditions reliably (timing, byte-exact thresholds, blocking under artificially low limits). Without these tests, a regression in _AppendPermits.acquire or _Semaphore could silently break backpressure, causing either unbounded memory growth or permanent deadlocks in production clients.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: tests/test_session.py
   Line: 746-845
   
   Comment:
   **Concurrency primitive tests have no integration test substitute**
   
   `_AppendPermits` and `_Semaphore` are internal concurrency primitives whose blocking/unblocking behaviour under byte-count and batch-count limits was tested by six dedicated tests here. Integration tests running against a real service cannot reproduce these conditions reliably (timing, byte-exact thresholds, blocking under artificially low limits). Without these tests, a regression in `_AppendPermits.acquire` or `_Semaphore` could silently break backpressure, causing either unbounded memory growth or permanent deadlocks in production clients.
   
   How can I resolve this? If you propose a fix, please make it concise.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
tests/test_session.py:1066-1090
**Non-mock unit tests removed alongside mock tests**

The three `test_connect_error_*` tests at the bottom of this file use no mocking at all — they construct real `ConnectError` instances and assert on `has_no_side_effects`. Similarly, `test_terminal_416_raises_read_unwritten` exercises `read_messages` with a real byte stream, and `test_frame_signal_reset_after_resend` directly calls `_run_attempt`. None of these require a live service. Removing them means that `has_no_side_effects`, its `ConnectionRefusedError` chain-walk, and the `_run_attempt` frame-signal reset path are no longer exercised by any test in the suite. If the intent is to replace mock-based tests with integration tests, these lightweight unit tests for pure logic could be kept as-is, since integration tests are unlikely to cover the exact error-chain and frame-signal edge cases.

### Issue 2 of 2
tests/test_session.py:746-845
**Concurrency primitive tests have no integration test substitute**

`_AppendPermits` and `_Semaphore` are internal concurrency primitives whose blocking/unblocking behaviour under byte-count and batch-count limits was tested by six dedicated tests here. Integration tests running against a real service cannot reproduce these conditions reliably (timing, byte-exact thresholds, blocking under artificially low limits). Without these tests, a regression in `_AppendPermits.acquire` or `_Semaphore` could silently break backpressure, causing either unbounded memory growth or permanent deadlocks in production clients.

_{Reviews (1): Last reviewed commit: "initial commit" | Re-trigger Greptile}