From 89ef6a97fd9e7fdb7bbfdf1f0abfcc501a517cad Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sat, 22 Aug 2026 00:57:46 -0400 Subject: [PATCH] [FIX] migration: exit 1 means findings, not failure, for the view fixer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Entry [5] never asked "Correct them?". The tool ran, printed its diagnosis, exited 1 -- and todo_upgrade_execute read that as a failure and reopened ITS error menu on top of ours. The menu looped and nothing was ever corrected. wait_at_error=False on both calls, as the COW tool next door already does. The fake in the test now models the real contract: it refuses to return a non-zero status when the flag is missing, exactly as the real one refuses by recursing into its prompt. --- FR --- L'entrée [5] ne posait jamais la question « Les corriger ? ». L'outil tournait, affichait son diagnostic, sortait 1 — et todo_upgrade_execute y lisait un échec et rouvrait SON menu d'erreur par-dessus le nôtre. Le menu tournait en rond et rien n'était corrigé. wait_at_error=False sur les deux appels, comme le fait déjà l'outil COW juste à côté. Le faux du test modèle désormais le vrai contrat : il refuse de rendre un statut non nul quand le drapeau manque, comme le vrai refuse en repartant dans son invite. Assisted-by: Claude Opus 5 --- script/todo/todo_upgrade.py | 11 ++++-- test/test_fix_view_type.py | 69 +++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index 2ace1bc..e88354a 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2956,8 +2956,14 @@ class TodoUpgrade: pour réparer ce qui empêche Odoo de démarrer ne servirait à rien. """ outil = os.path.join(PATH_MIGRATION_GLOBAL, "fix_view_type.py") + # `wait_at_error=False` est OBLIGATOIRE : pour cet outil, 1 veut + # dire « des trouvailles », pas « échec ». Sans ce drapeau, + # `todo_upgrade_execute` y lit une panne et rouvre SON menu + # d'erreur par-dessus le nôtre — mesuré : la question « Les + # corriger ? » n'était jamais posée et le menu tournait en rond. status, _cmd = self.todo_upgrade_execute( - f"{PYTHON_BIN} ./{outil} -d {database}" + f"{PYTHON_BIN} ./{outil} -d {database}", + wait_at_error=False, ) if status != 1: # 0 : rien à corriger. 2 : l'outil a échoué. Ni l'un ni @@ -2971,7 +2977,8 @@ class TodoUpgrade: if reponse not in ("y", "yes", "o"): return False status, _cmd = self.todo_upgrade_execute( - f"{PYTHON_BIN} ./{outil} -d {database} --apply" + f"{PYTHON_BIN} ./{outil} -d {database} --apply", + wait_at_error=False, ) return status == 0 diff --git a/test/test_fix_view_type.py b/test/test_fix_view_type.py index 43263b5..e28926f 100644 --- a/test/test_fix_view_type.py +++ b/test/test_fix_view_type.py @@ -241,6 +241,75 @@ class TestTheWiring(unittest.TestCase): ) +class TestTheToolIsCalledWithTheRightContract(unittest.TestCase): + """Pour cet outil, 1 veut dire « trouvailles », pas « échec ». + + `todo_upgrade_execute` ouvre SON menu d'erreur sur tout statut non + nul. Sans `wait_at_error=False`, ce menu se superpose au nôtre : la + question « Les corriger ? » n'est jamais posée, et l'on tourne en + rond. C'est arrivé en vrai, sur une migration bloquée. + """ + + def setUp(self): + from script.todo import auto_ask + from script.todo.todo_upgrade import TodoUpgrade + + self.auto_ask = auto_ask + self.vrai_ask = auto_ask.ask + self.obj = TodoUpgrade.__new__(TodoUpgrade) + self.appels = [] + + def faux(cmd, **kw): + self.appels.append((cmd, kw)) + statut = 0 if "--apply" in cmd else 1 + if statut and kw.get("wait_at_error", True): + # Ce que fait le VRAI : il n'en revient pas avec le + # statut, il repart dans son propre menu. + raise AssertionError( + "menu d'erreur imbriqué : wait_at_error manquant" + ) + return statut, cmd + + self.obj.todo_upgrade_execute = faux + self.obj.ask = lambda prompt, default="": "y" + + def tearDown(self): + self.auto_ask.ask = self.vrai_ask + + def test_the_report_call_does_not_reopen_the_error_menu(self): + with redirect_stdout(io.StringIO()): + self.obj.prompt_fix_view_type("db") + self.assertTrue(self.appels) + self.assertIs(self.appels[0][1].get("wait_at_error"), False) + + def test_the_apply_call_does_not_either(self): + with redirect_stdout(io.StringIO()): + self.obj.prompt_fix_view_type("db") + self.assertEqual(len(self.appels), 2) + self.assertIn("--apply", self.appels[1][0]) + self.assertIs(self.appels[1][1].get("wait_at_error"), False) + + def test_it_returns_true_only_when_the_apply_succeeded(self): + with redirect_stdout(io.StringIO()): + self.assertTrue(self.obj.prompt_fix_view_type("db")) + + def test_refusing_runs_no_apply(self): + self.obj.ask = lambda prompt, default="": "n" + with redirect_stdout(io.StringIO()): + self.assertFalse(self.obj.prompt_fix_view_type("db")) + self.assertEqual(len(self.appels), 1) + + def test_nothing_to_fix_asks_nothing(self): + demandes = [] + self.obj.ask = ( + lambda prompt, default="": demandes.append(prompt) or "y" + ) + self.obj.todo_upgrade_execute = lambda cmd, **kw: (0, cmd) + with redirect_stdout(io.StringIO()): + self.assertFalse(self.obj.prompt_fix_view_type("db")) + self.assertEqual(demandes, []) + + class TestTranslations(unittest.TestCase): def test_every_key_exists(self): with io.open(fvt.__file__, encoding="utf-8") as handle: