Give three skipped tests the isolation their skip reason asked for - #484
Open
JE-Chen wants to merge 1 commit into
Open
Give three skipped tests the isolation their skip reason asked for#484JE-Chen wants to merge 1 commit into
JE-Chen wants to merge 1 commit into
Conversation
Their own reason said what they needed: "needs subprocess isolation (see test_actions_menu_gui) ... skip until then". They cover real wiring — a file received on a WebRTC worker thread reaching the GUI thread through a queued signal rather than a thread-affine singleShot, and the admin console's thumbnail poll deleting its QThread each tick instead of leaking one per interval. Skipping was right at the time. Building the WebRTC panel or the admin console and then tearing a worker QThread down aborts the shared pytest process under offscreen Qt, and since deleteLater is a no-op until an event loop runs, the abort lands in some later unrelated file with no traceback. Run all three in one child process that writes a JSON verdict per check and exits without teardown, the shape test_actions_menu_gui already uses. Verdicts are ok / failed / unavailable, so a machine without the webrtc extra reports a skip while one that has it checks the wiring. Confirmed to have teeth rather than assumed: dropping thread.finished.connect(thread.deleteLater) turns the third verdict into "failed: the QThread outlived finish" and leaves the other two green.
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 1 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



test_r3_gui_thread_marshal.pycarried three@pytest.mark.skips whose ownreason named the fix:
They cover real wiring — that a file received on a WebRTC worker thread reaches
the GUI thread through a queued signal rather than a thread-affine
QTimer.singleShot, and that the admin console's thumbnail poll deletes itsQThreadeach tick instead of leaking one per interval.Skipping them was right at the time. Building the WebRTC panel or the admin
console and then tearing a worker
QThreaddown aborts the shared pytestprocess under offscreen Qt. And because
deleteLateris a no-op until an eventloop runs, the abort does not land in the test that caused it — it detonates
inside some later, unrelated file, with no traceback.
What changed
One probe runs all three checks in a child process, writes a JSON verdict per
check, and
os._exit(0)s without teardown — the same shapetest_actions_menu_guialready uses for the full tab set.Verdicts are
ok,failed: …orunavailable: …, so a machine without the[webrtc]extra (CI'spytest-headless) reports a skip, while a machinethat has it actually checks the wiring. That distinction matters: a blanket
importorskipwould have hidden a real failure on developer machines too.The checks have teeth — verified, not assumed
Deleting the single line
thread.finished.connect(thread.deleteLater)fromadmin_console_tab.py:…while
panel_signalsandwebrtc_marshalstayed green, so the failure islocalised to the wiring it covers.
Result
The headless suite now runs end to end with no
--ignoreflags — 4,815passing, exit 0. The 17 remaining skips are all optional-dependency or
platform gates (
Darwin-only path,no module named openpyxl, …). No "skipuntil then" is left in the suite.
Progress.mdloses the entry, per the rule that it records only open items.