mengw15 opened a new issue, #6899:
URL: https://github.com/apache/texera/issues/6899
### Task Summary
`LinkedBlockingMultiQueue`
(`amber/src/main/python/core/util/customized_queue/linked_blocking_multi_queue.py`)
is at ~70% — 86 tracked lines are unhit. The existing suite's 12 tests only
exercise the enqueue/dequeue happy path; the whole non-destructive read path
(`peek` at all three levels), the removal path (`SubQueue.clear`,
`SubQueue.remove`/`unlink`, `PriorityGroup.remove_queue`, `remove_sub_queue`)
and several small guards have no test at all. Every uncovered line is pure — no
threads, sleeps, sockets or blocking waits are needed, because the one
condition-variable wait in the file lives in `get()` and is already covered.
### Behavior to add
Extend
`amber/src/test/python/core/util/customized_queue/test_linked_blocking_multi_queue.py`,
reusing its existing `queue` fixture (`control` at priority 0, `data` at
priority 1). New tests are single-threaded, so unlike the existing thread-based
cases they need no `@pytest.mark.timeout`.
**`peek` (all three levels)**
- `LinkedBlockingMultiQueue.peek()` → `None` on a fresh queue; after `put`,
returns the item **without** changing `size()`; returns the priority-0
`control` item ahead of `data`; returns `None` once the only non-empty
sub-queue is disabled. Note it never calls `not_empty.wait()` — it
short-circuits on `total_count == 0`, which is why this is safe to test
directly.
- `PriorityGroup.peek` → first enabled non-empty queue's item, `None` when
all are empty. It mutates `next_idx` while skipping, so `peek` perturbs
round-robin state — assert that, it is surprising.
- `DefaultSubQueueSelection.peek` → first non-`None` across the priority
groups in order.
**`SubQueue.clear`** — zeroes `count`, drops `owner.total_count` by exactly
the old count, collapses the chain so `head is last`, and leaves `is_empty()`
true. With the sub-queue disabled first, `total_count` must not move.
`in_mem_size` is **not** reset by `clear` — pin the current behavior.
**`SubQueue.remove` / `unlink`** — `remove(None)` and a missing item both
return `False`. For a present item, the current implementation matches on the
node holding the item but unlinks the *following* node: on `["a","b","c"]`,
`remove("a")` returns `True` and leaves the chain as `[None, None, "c"]`, and
`self.count` is not decremented while `owner.total_count` is, so `sub.size()`
and `q.size()` disagree afterwards. Cover the `self.last == next_` fix-up and
the `if self.enabled` guard too.
**`PriorityGroup.remove_queue`** — the group shrinks, `next_idx` resets to 0
when it equalled `len(self.queues)`, and a non-matching key is a no-op.
**`remove_sub_queue`** — a missing key returns `None` and mutates nothing;
an existing key returns the `SubQueue`, drops it from `sub_queues`, and drops
the now-empty `PriorityGroup` from `priority_groups`.
**Small guards** — `SubQueue.is_empty`, `SubQueue.__str__` (`"a -> b -> c ->
"`, `""` when empty; only meaningful for `str` items), `put(key, None)` →
`ValueError("Does not support NoneType.")`,
`DefaultSubQueueSelection.get_next()` → `None` when everything is empty (call
it directly; going through `q.get()` on an empty queue would block forever),
`set_priority_groups([])`, and `__len__`.
### Two defects the coverage work runs into
Both are pre-existing and are why parts of this file were never exercised.
Either fix them in the same PR (xinyuan's #6878 is the precedent for
fix-plus-tests) or pin current behavior and file them separately — but do not
leave them silently encoded as "expected".
1. **`SubQueue.remove` corrupts the chain and desynchronises `count`**, as
described above.
2. **`remove_sub_queue` raises `AttributeError` for any *enabled*
sub-queue.** `PriorityGroup` instances are built through class access —
`LinkedBlockingMultiQueue.PriorityGroup(priority)` — whereas `SubQueue` uses
instance access (`self.SubQueue(key)`). The `@inner` descriptor only rebinds
`owner` to the outer instance on instance access, so `PriorityGroup.owner`
stays the class and `self.owner.total_count.get_and_dec(...)` in `remove_queue`
fails with `type object 'LinkedBlockingMultiQueue' has no attribute
'total_count'`. Without the one-token fix, that line stays unreachable and the
test must disable the sub-queue first.
### Conventions
`amber/pyproject.toml` registers only the `integration` marker — unit tests
are the default, unmarked state, so do **not** add `@pytest.mark.unit`. Ruff
gates the diff at `line-length = 88`, `target-version = "py311"`. Keep the ASF
header. Group new cases into `class TestXxx:` blocks like the rest of the file.
### Task Type
- [ ] Refactor / Cleanup
- [ ] DevOps / Deployment / CI
- [x] Testing / QA
- [ ] Documentation
- [ ] Performance
- [ ] Other
--
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]
