MIL-009 task 9: fix a flaky unit test that created a real Tk root #66
@@ -25,7 +25,7 @@ document of a type. `Primary File` may contain a glob (e.g.
|
|||||||
| DCD | Design Class Diagram | docs/dcd.md | 002 |
|
| DCD | Design Class Diagram | docs/dcd.md | 002 |
|
||||||
| DICT | Domain Dictionary | docs/dictionary.md | 002 |
|
| DICT | Domain Dictionary | docs/dictionary.md | 002 |
|
||||||
| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0005 |
|
| ADR | Architecture Decision Record | docs/adr/adr-NNNN-*.md | 0005 |
|
||||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 049 |
|
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 051 |
|
||||||
| TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 |
|
| TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 |
|
||||||
|
|
||||||
## Languages
|
## Languages
|
||||||
|
|||||||
@@ -11,7 +11,8 @@
|
|||||||
## Version History
|
## Version History
|
||||||
| Date | Status | Author | Reviewer | Change | Commit |
|
| Date | Status | Author | Reviewer | Change | Commit |
|
||||||
| --- | --- | --- | --- | --- | --- |
|
| --- | --- | --- | --- | --- | --- |
|
||||||
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [768ca34] |
|
| 2026-10-15 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [768ca34] |
|
||||||
|
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added task 9 (fix a flaky unit test found after the merge) | [061b4f4] |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -73,6 +74,7 @@ Decide whether the Learner can start the program from one executable file withou
|
|||||||
| 6 | Build and smoke-test the executable | Build it, start it with the virtual environment not active, check the window, the images, the level choice, the progress file next to it, the word list override and the speech import; record the result in the review record. | Yes | US-001.07 |
|
| 6 | Build and smoke-test the executable | Build it, start it with the virtual environment not active, check the window, the images, the level choice, the progress file next to it, the word list override and the speech import; record the result in the review record. | Yes | US-001.07 |
|
||||||
| 7 | Test the path rules and the build arguments | Unit tests for the frozen and source path rules and for the argument list the build script gives PyInstaller (without running PyInstaller). | No | |
|
| 7 | Test the path rules and the build arguments | Unit tests for the frozen and source path rules and for the argument list the build script gives PyInstaller (without running PyInstaller). | No | |
|
||||||
| 8 | Update README and wiki | README and the Install page of the wiki in English, Danish and Bahasa Malaysia: how to build, where the executable and the data folder are, that Python is not needed to run it. The wiki is republished after the merge. | No | |
|
| 8 | Update README and wiki | README and the Install page of the wiki in English, Danish and Bahasa Malaysia: how to build, where the executable and the data folder are, that Python is not needed to run it. The wiki is republished after the merge. | No | |
|
||||||
|
| 9 | Fix a flaky unit test | After the merge a unit test of `app.main` failed in about 1 of 6 full runs because it created a real Tk root; it uses the module's fake root instead, so the unit tests never need a display. Found by running the suite repeatedly. | No | RC-049 |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -81,3 +83,4 @@ Decide whether the Learner can start the program from one executable file withou
|
|||||||
[UC-001]: ../uc-001/uc.md
|
[UC-001]: ../uc-001/uc.md
|
||||||
[DCD-001]: ../dcd.md
|
[DCD-001]: ../dcd.md
|
||||||
[768ca34]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/768ca344c95d4f21e565e37ac96661c2f8dc8f54
|
[768ca34]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/768ca344c95d4f21e565e37ac96661c2f8dc8f54
|
||||||
|
[061b4f4]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/061b4f48b5d49e6800656c95d79df707e07ec0f3
|
||||||
|
|||||||
@@ -0,0 +1,64 @@
|
|||||||
|
# RC-049: Review of PY-FIX-001
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
| Key | Value |
|
||||||
|
| --- | --- |
|
||||||
|
| ID | RC-049 |
|
||||||
|
| CrossReference | [PY-FIX-001], [QC-PY-001] |
|
||||||
|
|
||||||
|
## Version History
|
||||||
|
| Date | Status | Author | Reviewer | Change | Commit |
|
||||||
|
| --- | --- | --- | --- | --- | --- |
|
||||||
|
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [061b4f4] |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Artifact Under Review
|
||||||
|
|
||||||
|
- Instance reviewed: [PY-FIX-001]
|
||||||
|
- Checklist used: [QC-PY-001]
|
||||||
|
- Scope: full review
|
||||||
|
- Language and domain: n/a (technical type)
|
||||||
|
- Language reviewer: none
|
||||||
|
- Mode: autonomous, not independent (reviewer is `AI as stand-in for S01`)
|
||||||
|
|
||||||
|
## Checklist Results
|
||||||
|
|
||||||
|
| # | Criterion | Status | Evidence/Notes |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | The fixture and names follow PEP 8 (`fake_tk_root`). |
|
||||||
|
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose. |
|
||||||
|
| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | `ruff check` and `ruff format --check` pass with no suppressions. |
|
||||||
|
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | The fixture is annotated `-> None`; `mypy --strict` passes on 45 source files. |
|
||||||
|
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | No exception handling in the change. |
|
||||||
|
| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults. |
|
||||||
|
| 7 | Files, locks and connections are managed with context managers | N-A | No file handling in the change. |
|
||||||
|
| 8 | Public modules, classes and functions have docstrings that say what, not how | N-A | Tests are excluded from the docstring rules; the fixture has a one-line docstring. |
|
||||||
|
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | No `print`, no logging in the change. |
|
||||||
|
| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | Pass | No class or operation changed; the change is in a test of `app.main`. |
|
||||||
|
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | The change makes the test independent of a real Tk root: 20 consecutive full runs (216 tests) were green after 2 failures in 12 runs before; real Tk stays covered by `tests/e2e`. |
|
||||||
|
| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | `mypy --strict` reports no issue. |
|
||||||
|
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | No dependency change. |
|
||||||
|
| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | The fake root is the module's existing `_FakeRoot`, used once by an autouse fixture instead of per test. |
|
||||||
|
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | No source import changed; `lint-imports` kept. |
|
||||||
|
| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | One fixture with one responsibility. |
|
||||||
|
| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Test file stays in `tests/unit`. |
|
||||||
|
| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | File names unchanged. |
|
||||||
|
|
||||||
|
## Overall Verdict
|
||||||
|
|
||||||
|
Go — every Mandatory criterion passes. Reviewed by AI as stand-in for S01; not independent of the author.
|
||||||
|
|
||||||
|
Defect fixed: an unpatched `app.main` call created a real `tk.Tk()` inside a unit test, which failed in about 1 of 6 full runs (the only Tk process limit and Tcl handle problem seen in MIL-005). Found after the MIL-009 merge, outside any milestone task, so no issue is closed. Go.
|
||||||
|
|
||||||
|
## Action Items
|
||||||
|
|
||||||
|
| Action | Owner | Due |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| none | - | - |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
[PY-FIX-001]: ../../../tests/unit/test_app.py
|
||||||
|
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
|
||||||
|
[061b4f4]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/061b4f48b5d49e6800656c95d79df707e07ec0f3
|
||||||
@@ -0,0 +1,68 @@
|
|||||||
|
# RC-050: Review of MIL-009
|
||||||
|
|
||||||
|
## Metadata
|
||||||
|
| Key | Value |
|
||||||
|
| --- | --- |
|
||||||
|
| ID | RC-050 |
|
||||||
|
| CrossReference | [MIL-009], [QC-MIL-001], [QC-LANG-001] |
|
||||||
|
|
||||||
|
## Version History
|
||||||
|
| Date | Status | Author | Reviewer | Change | Commit |
|
||||||
|
| --- | --- | --- | --- | --- | --- |
|
||||||
|
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [061b4f4] |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Artifact Under Review
|
||||||
|
|
||||||
|
- Instance reviewed: [MIL-009]
|
||||||
|
- Checklist used: [QC-MIL-001], and [QC-LANG-001] (language and domain)
|
||||||
|
- Scope: full review
|
||||||
|
- Language and domain: en / it
|
||||||
|
- Language reviewer: none (the stand-in for S01 reads the language and knows the domain)
|
||||||
|
- Mode: autonomous, not independent (reviewer is `AI as stand-in for S01`)
|
||||||
|
|
||||||
|
## Checklist Results
|
||||||
|
|
||||||
|
| # | Criterion | Status | Evidence/Notes |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| 1 | A concrete deliverable is defined for every gate | Pass | A concrete Deliverable section names the output evaluated at the gate. |
|
||||||
|
| 2 | Explicit Go/No-Go criteria are stated for each gate | Pass | The Go / No-Go table has numbered criteria, each with an observable Go and No-Go. |
|
||||||
|
| 3 | Dependencies on other milestones are explicitly mapped | Pass | Dependencies table names the milestone or documents it needs. |
|
||||||
|
| 4 | Each milestone is traceable to a Business Case objective or KPI | Pass | Traceability table maps to Business Case objectives and the user story. |
|
||||||
|
| 5 | Milestone owner and approving reviewer are identified | Pass | Ownership table names S01 as owner and approving reviewer. |
|
||||||
|
| 6 | Milestone has a defined target date consistent with project constraints | Pass | Target Date lies inside the 2026-10-14 to 2026-10-21 window of the Business Case. |
|
||||||
|
|
||||||
|
## Language and Domain Results
|
||||||
|
|
||||||
|
| # | Criterion | Status | Evidence/Notes |
|
||||||
|
| --- | --- | --- | --- |
|
||||||
|
| 1 | The Metadata table has a `Language` row and a `Domain` row, and neither is a placeholder | Pass | Metadata of MIL-009 has `Language | en` and `Domain | it`; neither is a placeholder. |
|
||||||
|
| 2 | `Language` is a BCP 47 code and `Domain` is a value from the registry's domain list | Pass | `en` is a BCP 47 code and `it` is a value of the registry's domain list. |
|
||||||
|
| 3 | The content (prose and table cells) is written in the stated language | Pass | Prose and table cells are English; Malay and Danish words and file names appear only as quoted data. |
|
||||||
|
| 4 | The register matches the one the registry gives for the artifact type | Pass | The registry gives `IT Executive English` for this type; the text is written for that reader. |
|
||||||
|
| 5 | Domain terms are the PO terms of the domain's dictionary, with no synonyms | Pass | The PO terms of DICT-001 (flash card, word list, words to learn, study session) are used and no synonym replaces them. |
|
||||||
|
| 6 | Metadata keys, section headings, IDs and statuses are in English | Pass | Metadata keys, headings, IDs and statuses are English. |
|
||||||
|
| 7 | No translated twin (`<name>.<language>.md`) exists beside the document | Pass | No translated twin exists beside the document (checked in `docs/`). |
|
||||||
|
| 8 | A change of language or domain since the previous accepted version has a Version History row and was reviewed again | Pass | First version; no change of language or domain since an earlier accepted version. |
|
||||||
|
| 9 | A reviewer competent in the domain, and in the language, has confirmed that the domain terms are used correctly | Pass | The reviewer is the stand-in for S01, the Product Owner, who reads English and knows the IT domain; no second language reviewer is needed (review-checklist-process step 2). Open to challenge by S01 in the end-of-run summary. |
|
||||||
|
| 10 | Abbreviations are spelled out on first use, in the stated language | Fail | CSV, MVVM, UI, PO and similar abbreviations are not all spelled out at first use; Optional, deferred. |
|
||||||
|
|
||||||
|
## Overall Verdict
|
||||||
|
|
||||||
|
Go — every Mandatory criterion passes; the failed Optional criteria are deferred with an action item. Reviewed by AI as stand-in for S01; not independent of the author.
|
||||||
|
|
||||||
|
Re-review of MIL-009 after task 9 was added (a flaky unit test fix); Go/No-Go criteria, deliverable and dates are unchanged.
|
||||||
|
|
||||||
|
## Action Items
|
||||||
|
|
||||||
|
| Action | Owner | Due |
|
||||||
|
| --- | --- | --- |
|
||||||
|
| Criterion LANG 10 (Optional) failed: Abbreviations are spelled out on first use, in the stated language; fix or accept the deferral | S01 | 2026-10-21 |
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
[MIL-009]: ../../../docs/milestones/mil-009-executable.md
|
||||||
|
[QC-MIL-001]: ../../../framework/qc/artifacts/qc-milestones-gateways.md
|
||||||
|
[QC-LANG-001]: ../../../framework/qc/artifacts/qc-language-domain.md
|
||||||
|
[061b4f4]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/061b4f48b5d49e6800656c95d79df707e07ec0f3
|
||||||
@@ -11,7 +11,8 @@
|
|||||||
| --- | --- | --- | --- | --- | --- |
|
| --- | --- | --- | --- | --- | --- |
|
||||||
| 2026-10-14 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [c3fbcbc] |
|
| 2026-10-14 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [c3fbcbc] |
|
||||||
| 2026-10-14 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added MIL-008 and the review records of the level choice (no checklist exists for this type) | [0b983b7] |
|
| 2026-10-14 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added MIL-008 and the review records of the level choice (no checklist exists for this type) | [0b983b7] |
|
||||||
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added the rows and review records of the executable request (no checklist exists for this type) | [768ca34] |
|
| 2026-10-15 | Deprecated | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added the rows and review records of the executable request (no checklist exists for this type) | [768ca34] |
|
||||||
|
| 2026-10-15 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Added the rows and review records of the executable request (no checklist exists for this type) | [061b4f4] |
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@@ -48,7 +49,7 @@ updated whenever an artifact instance is created or reviewed.
|
|||||||
| MIL-006 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-006 | [RC-021] |
|
| MIL-006 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-006 | [RC-021] |
|
||||||
| MIL-007 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-007 | [RC-022] |
|
| MIL-007 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-007 | [RC-022] |
|
||||||
| MIL-008 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-008 | [RC-040] |
|
| MIL-008 | Milestone / Gateway | en | it | BC-001, PP-001 | tasks of MIL-008 | [RC-040] |
|
||||||
| MIL-009 | Milestone / Gateway | en | it | BC-001 | - | [RC-047] |
|
| MIL-009 | Milestone / Gateway | en | it | BC-001 | - | [RC-050] |
|
||||||
| ADR-0004 | Architecture Decision Record | - | - | BC-001 | - | [RC-046] |
|
| ADR-0004 | Architecture Decision Record | - | - | BC-001 | - | [RC-046] |
|
||||||
| PY-MIL-001 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-023] |
|
| PY-MIL-001 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-023] |
|
||||||
| PY-MIL-002 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-024] |
|
| PY-MIL-002 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-024] |
|
||||||
@@ -59,6 +60,7 @@ updated whenever an artifact instance is created or reviewed.
|
|||||||
| PY-MIL-007 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-029] |
|
| PY-MIL-007 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-029] |
|
||||||
| PY-MIL-008 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-041] |
|
| PY-MIL-008 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-041] |
|
||||||
| PY-MIL-009 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-048] |
|
| PY-MIL-009 | Python Source Code | - | - | DCD-001, ADR-0002 | - | [RC-048] |
|
||||||
|
| PY-FIX-001 | Python Source Code | - | - | PY-MIL-008 | - | [RC-049] |
|
||||||
|
|
||||||
## Coverage Notes
|
## Coverage Notes
|
||||||
|
|
||||||
@@ -153,3 +155,7 @@ updated whenever an artifact instance is created or reviewed.
|
|||||||
[768ca34]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/768ca344c95d4f21e565e37ac96661c2f8dc8f54
|
[768ca34]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/768ca344c95d4f21e565e37ac96661c2f8dc8f54
|
||||||
[RC-048]: ./reviews/rc-048-py-mil-009.md
|
[RC-048]: ./reviews/rc-048-py-mil-009.md
|
||||||
[PY-MIL-009]: ../../src/flash_card
|
[PY-MIL-009]: ../../src/flash_card
|
||||||
|
[RC-049]: ./reviews/rc-049-py-fix-001.md
|
||||||
|
[PY-FIX-001]: ../../tests/unit/test_app.py
|
||||||
|
[RC-050]: ./reviews/rc-050-mil-009-task9.md
|
||||||
|
[061b4f4]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/031-flash-card/commit/061b4f48b5d49e6800656c95d79df707e07ec0f3
|
||||||
|
|||||||
@@ -92,6 +92,12 @@ def _first(cards: Sequence[Card]) -> Card:
|
|||||||
return cards[0]
|
return cards[0]
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture(autouse=True)
|
||||||
|
def fake_tk_root(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""Keep every test in this module away from a real Tk root."""
|
||||||
|
monkeypatch.setattr(app.tk, "Tk", _FakeRoot)
|
||||||
|
|
||||||
|
|
||||||
@pytest.fixture
|
@pytest.fixture
|
||||||
def fake_views(monkeypatch: pytest.MonkeyPatch) -> None:
|
def fake_views(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
monkeypatch.setattr(app, "LevelView", _FakeLevelView)
|
monkeypatch.setattr(app, "LevelView", _FakeLevelView)
|
||||||
@@ -280,7 +286,6 @@ def test_create_application_draws_first_card_from_bundled_list(
|
|||||||
bundled.parent.mkdir()
|
bundled.parent.mkdir()
|
||||||
bundled.write_text("ms;da\nsaya;jeg\n", encoding="utf-8")
|
bundled.write_text("ms;da\nsaya;jeg\n", encoding="utf-8")
|
||||||
data = tmp_path / "data"
|
data = tmp_path / "data"
|
||||||
monkeypatch.setattr(app.tk, "Tk", _FakeRoot)
|
|
||||||
monkeypatch.setattr(app, "DEFAULT_DATA_DIR", data)
|
monkeypatch.setattr(app, "DEFAULT_DATA_DIR", data)
|
||||||
monkeypatch.setattr(app, "BUNDLED_VOCABULARY_PATH", bundled)
|
monkeypatch.setattr(app, "BUNDLED_VOCABULARY_PATH", bundled)
|
||||||
application = app.create_application(None, False, Level(None))
|
application = app.create_application(None, False, Level(None))
|
||||||
|
|||||||
Reference in New Issue
Block a user