mengw15 opened a new pull request, #6903:
URL: https://github.com/apache/texera/pull/6903
### What changes were proposed in this PR?
`LinkedBlockingMultiQueue` sits at ~70% line coverage, and writing tests for
the untested parts surfaced two real defects — both on the removal paths, which
is why those lines were never exercised. This fixes both and adds the tests
that pin the corrected behavior.
**`SubQueue.remove` corrupted the chain and desynchronised the count.**
`head` is a sentinel, so the candidate node is `trail.next`, but `remove`
matched `trail.item` (the predecessor's own item) and then unlinked
`trail.next`; `unlink` in turn cleared `trail.item` instead of the removed
node's, and never decremented `self.count`. On `["a", "b", "c"]`, `remove("a")`
returned `True` but left a `None` in place of `"a"` and dropped `"b"` entirely,
left `SubQueue.size()` at 3 while the queue total went to 2, and made the next
`get()` return `None` instead of an item. Fixed by matching `trail.next.item`,
clearing `next_.item`, and decrementing `self.count` in `unlink`.
**`remove_sub_queue` raised `AttributeError` for any enabled sub-queue.**
`add_sub_queue` constructed priority groups through class access
(`LinkedBlockingMultiQueue.PriorityGroup(...)`), and the `@inner` descriptor
only rebinds `owner` to the outer instance on instance access — so
`PriorityGroup.owner` stayed the class and
`self.owner.total_count.get_and_dec(...)` in `remove_queue` failed with `type
object 'LinkedBlockingMultiQueue' has no attribute 'total_count'`. Fixed by
constructing through `self.PriorityGroup(...)`, matching how `SubQueue` is
already built. `Node` and `DefaultSubQueueSelection` are also built through
class access but never read `self.owner`, so they are left unchanged.
Neither method is reachable from production code today —
`internal_queue.py`, the only caller of this class, uses
`put`/`get`/`size`/`enable`/`disable`/`is_empty`/`is_enabled`/`in_mem_size`/`add_sub_queue`
— so no existing caller changes behavior.
The spec grows by 30 tests covering `peek` at all three levels,
`SubQueue.clear`, the corrected removal paths, `PriorityGroup.remove_queue`,
`remove_sub_queue`, and the small guards; the `queue` fixture moved to module
scope so the new classes share it. Two current behaviors are pinned rather than
changed: `clear()` does not reset `in_mem_size`, and `SubQueue.__str__` only
supports `str` items.
### Any related issues, documentation, discussions?
Closes #6899.
### How was this PR tested?
The target file passes with 42 tests (12 existing, 30 new), and the full
`pytest -m "not integration"` suite passes with no regressions; `ruff check`
and `ruff format --check` are clean. Both defects were reproduced before the
fix and confirmed gone after. For the failure path, each fix was reverted in
turn: reverting the `remove` fix reddens 4 removal tests, and reverting the
owner-binding fix brings back the original `AttributeError` — so the new tests
genuinely guard both fixes.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Fable 5)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
