Skip to content

fix(tests): issue-147 cwd fault hits only the grading fallback (#692) - #954

Merged
github-actions[bot] merged 1 commit into
mainfrom
fix/692-grading-cwd-fault-scope
Nov 9, 2026
Merged

fix(tests): issue-147 cwd fault hits only the grading fallback (#692)#954
github-actions[bot] merged 1 commit into
mainfrom
fix/692-grading-cwd-fault-scope

Conversation

@cdeust

@cdeust cdeust commented Nov 9, 2026

Copy link
Copy Markdown
Owner

Summary

Tranche de la classe 9 de #692 (« Path.resolve mocké », 2 tests). Le run Windows 38008915302 (job 114084097957) faisait échouer test_force_write_survives_missing_cwd et test_force_write_tags_grading_failure_observably avec un FileNotFoundError. L'erreur venait de <frozen ntpath>:719, dans le Path(db).resolve() du store, avant que l'écriture n'atteigne la notation.

Cause racine (défaut du test, pas du produit) : les tests remplaçaient os.getcwd pour tout l'interpréteur. Or, sous Windows, ntpath.realpath lit le cwd même pour un chemin absolu. De plus, Windows refuse de supprimer le cwd d'un processus, donc cet échec n'existe que sous le mock. Le défaut est maintenant injecté là où l'issue 147 l'a localisé : le repli os.getcwd() de handlers/remember_helpers._grade_content_best_effort, via patch.object(remember_helpers, "os", Mock(wraps=os, getcwd=…)). Aucun code de production n'est touché.

Refs #692

Type of change

  • Bug fix (non-breaking change which fixes an issue), dans les tests uniquement

Test plan

  • pytest tests_py/handlers/test_issue_147_write_class_and_grading.py : 13 passed (Linux, SQLite).
  • pytest tests_py/handlers : 1937 passed, 81 skipped.
  • Simulation de ntpath.realpath : un plugin pytest fait lire os.getcwd() par Path.resolve, comme le fait Windows. Avec cette simulation, l'ancien test donne 2 failed, ce qui reproduit les 2 échecs Windows. Le nouveau test passe (4/4 sur la classe).
  • Mutation : si le getcwd mocké renvoie un chemin au lieu de lever une exception, test_force_write_tags_grading_failure_observably échoue. Le défaut est donc bien atteint. test_force_write_survives_missing_cwd seul ne tue pas ce mutant, parce qu'un contenu sans référence est noté unverifiable dans les deux cas.
  • ruff check / ruff format --check (ruff 0.17.3), python scripts/check_craftsmanship.py : OK. Le fichier fait 295 lignes, sous le plafond de 300.

Non vérifié :

Audit notes

  • Revue automatisée (routine autopilot), pas de revue humaine à ce stade.

Coding-standards compliance

  • §2.2 couches inchangées (test uniquement).
  • §9 pas de code mort.

Reviewer checklist

  • CHANGELOG.md mis à jour (Unreleased → Fixed).
  • Pas de secret dans le diff.

Generated by Claude Code

@claude
fix(tests): issue-147 cwd fault hits only the grading fallback (#692)
2e27ef4 
Windows run 38008915302 (job 114084097957) failed
test_force_write_survives_missing_cwd and
test_force_write_tags_grading_failure_observably with FileNotFoundError
raised from <frozen ntpath>:719 inside the store's Path(db).resolve(),
before the write reached grading. The tests patched os.getcwd for the
whole interpreter, and on Windows ntpath.realpath reads the cwd even for
an absolute path. Windows refuses to remove a process's cwd, so that
failure exists only under the mock. The fault is now injected where
issue 147 located it: the os.getcwd() fallback in
handlers/remember_helpers._grade_content_best_effort.

Refs #692

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_018kyZYsMMHWM8R2NFVyNfFC
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T22:33:56.574475Z 2e27ef4 PR opened
🔒 Security Review ✅ Completed 2026-11-09T22:34:56.124583Z 2e27ef4 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cdeust cdeust mentioned this pull request Nov 9, 2026
Open
@claude

claude Bot commented Nov 9, 2026 •
edited
Loading

Copy link
Copy Markdown

Claude finished @cdeust's task in 1m 48s —— View job

PR #954 review: done

No Blockers. The review lists 3 Notes, none needed before merge. I couldn't run pytest in this job because the command needed approval, so the verdict comes from reading the code.

Verdict: approve

@claude

claude Bot commented Nov 9, 2026

Copy link
Copy Markdown

Review 1 of 3 (ADR-1119): PR #954

Five reviews: code quality, test coverage, docs accuracy, security and performance. No Blockers.

  • The patch targets the right global. The handler path (remember.py → insert_and_post_process → _grade_content_best_effort) reaches remember_helpers.os.getcwd() at remember_helpers.py:647 with an empty directory. The only other os uses in that module are os.path.isdir at lines 633 and 654, and they still work through Mock(wraps=os).
  • The changelog and docstring claims hold up. CPython's ntpath.realpath calls os.getcwd() before its isabs check. The store reaches Path(path).resolve() via sqlite_connection_registry.py:191.
  • The tests use isolated temp databases. There are no secrets, and the file is 296 of 300 lines.

Notes, no action before merge

  • test_force_write_survives_missing_cwd would pass even without the fault. Adding getcwd.assert_called_once() would make it fail when the fault isn't reached.
  • test_issue_147_write_class_and_grading.py:178 still uses a global patch("os.getcwd"). It's safe because it calls the helper directly, but _cwd_gone_at_grading() there would keep one style.
  • At 296 of 300 lines, the next addition to this file will likely need a split.

I couldn't run pytest in this job (the command needed approval), so this verdict comes from reading the code.

Verdict: approve

@cdeust Claude

cdeust commented Nov 9, 2026

Copy link
Copy Markdown
Owner Author

Réponse aux trois notes non bloquantes de la revue 1. Aucune ne justifie de pousser un commit seul :

  • assert_called_once() : le mutant « getcwd ne lève pas » est déjà tué par test_force_write_tags_grading_failure_observably (vérifié localement). Il survit seulement dans test_force_write_survives_missing_cwd. Je l'ajouterai si un autre commit doit être poussé sur cette branche.
  • Le patch("os.getcwd") global de la ligne 178 appelle le helper directement, sans resolve() du store. Il passait sous Windows dans le run 38008915302, et le changer élargirait la PR au-delà des 2 échecs.
  • Il reste 4 lignes sous le plafond de 300 : c'est noté pour la prochaine tranche qui touchera ce fichier.

Generated by Claude Code

@github-actions
github-actions Bot merged commit c95fab2 into main Nov 9, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@cdeust @claude