Fix two flaky tests: subdeck flashcard-selector button and reminder dialog#1301
Draft
RisingOrange wants to merge 3 commits into
Draft
Fix two flaky tests: subdeck flashcard-selector button and reminder dialog#1301RisingOrange wants to merge 3 commits into
RisingOrange wants to merge 3 commits into
Conversation
…ialog - test_flashcard_selector_button_exists_for_subdeck_of_deck_with_note_embeddings: replace fixed qtbot.wait(500) + one-shot DOM check with qtbot.wait_until polling. The button is injected asynchronously after overview_did_refresh and the fixed wait is unreliable under CI load. - TestShowNextDueDateReminderDialog: add class-scoped autouse fixture that clears _reminder_dialog_state and drains pending QTimer.singleShot callbacks before each test's @patch decorators activate. Real-dialog tests connect dialog.finished -> singleShot(0, _show_next_due_date_reminder_dialog), and a stray late timer was firing inside this test's mock window and recording a SubdeckDueDateReminderDialog(parent=<real AnkiQt>) call on the mocked dialog class, making the assert_called_once_with(parent=mock_aqt.mw) check fail.
Codex pointed out that TestShowSubdeckDueDateReminders also touches _reminder_dialog_state and leaves the queue populated (it mocks out _show_next_due_date_reminder_dialog, so the queue never drains). A late real QTimer.singleShot firing afterwards would then act on real aqt with that residue. Pull the setup/teardown out of TestShowNextDueDateReminderDialog into a module-level fixture and apply it to both reminder-dialog test classes via pytest.mark.usefixtures. Also add the symmetric post-yield teardown reset so each test cleans up after itself.
The inline closure in the subdeck flake fix was a copy-pastable shape; lift it to a module-level helper so future "wait until DOM element exists" tests have something to reach for. Tried adopting it in test_flashcard_selector_send_note_suspension_states_message, but that test depends on the original qtbot.wait(500) to let the webview's _domDone flip true before any evalWithCallback runs. Polling immediately queues an eval that sits there until the wait_callback times out at 5s. Left that test on its qtbot.wait(500) — it isn't flaky and isn't worth destabilising for the refactor.
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
1 participant
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.
Related issues
N/A — flakes spotted on the NRT-749 / 751 / 763 / 764 CI runs. Findings written up in
.claude/worktrees/NRT-749-qa-followup/notes/nrt_flaky_tests.md.Proposed changes
Two unrelated CI flakes, fixed in one PR off
mainso they don't get tied to any in-flight feature branch.1.
tests/addon/test_integration.py—test_flashcard_selector_button_exists_for_subdeck_of_deck_with_note_embeddingsReplace fixed
qtbot.wait(500)+ one-shotevalWithCallbackDOM check withqtbot.wait_untilpolling. The flashcard-selector button is injected asynchronously afterset_current_deckfiresoverview_did_refresh, and the subdeck path is a touch slower than the parent-deck path because it walks ancestors to find the AnkiHub deck. A fixed 500ms wait is unreliable under CI load.2.
tests/addon/test_unit.py—TestShowNextDueDateReminderDialogAdd a class-scoped autouse fixture that clears
_reminder_dialog_state.queue / .dialogand drains pendingQTimer.singleShotcallbacks before each test's@patchdecorators activate.Root cause: real-dialog tests connect
dialog.finished→QTimer.singleShot(0, _show_next_due_date_reminder_dialog). A stray late timer was firing inside this unit test's mock window and recording aSubdeckDueDateReminderDialog(parent=<real AnkiQt>)call on the mocked dialog class, makingassert_called_once_with(parent=mock_aqt.mw)fail with the realaqt.mwas the parent. Clearing the queue + drainingprocessEvents()before mocks bind means any stray timer hits the empty-queue early-return without touching the mocks.Not in this PR: a single sighting of
tests/addon/performance/test_subdeck_operations.py::TestFlattenDeck::test_basicflaked once on NRT-751 — not investigated; revisit if it recurs.How to reproduce
These are CI flakes; locally they pass consistently. To verify the fixes:
Both pass; the subdeck integration test runs in ~1.3s.
Screenshots and videos
N/A — test-only change.
Further comments
Intentionally scoped: only the two flakes I could root-cause. The flatten-deck performance test gets a fresh look if it shows up again.