From 809273eb345e6e569220c31b8fc6bed8a5119d78 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Fri, 28 Aug 2026 01:09:24 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20verdicts=20de=20migration=20:=20disting?= =?UTF-8?q?uer=20trouvaille=20et=20=C3=A9chec=20d'outil?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit L'écran peignait en échec tout code non nul, quand la convention écrite dans todo_upgrade.run_tool dit : 0 rien à signaler, 1 des trouvailles, 2 l'outil a échoué. database_cleanup imprime lui-même « This is a warning, not a failure » avant de rendre 1. Un écran qui contredit l'outil apprend à ignorer les deux : du rouge signalait une migration en échec là où rien n'avait échoué. L'icône rejoint la couleur dans migration_status, et le panneau écrit le sens du chiffre à côté de lui : « statut 1 (des trouvailles) ». Testé. --- EN --- The screen painted every non-zero code as a failure, where the convention written in todo_upgrade.run_tool says: 0 nothing to report, 1 findings, 2 the tool failed. database_cleanup itself prints "This is a warning, not a failure" before returning 1. A screen that contradicts the tool teaches you to ignore both: red marked a migration as failed where nothing had failed. The icon now lives beside the colour in migration_status, and the panel spells the number out: "status 1 (findings)". Covered by tests. Assisted-by: Claude Opus 5 (cherry picked from commit 39d122965e7a09d710255fff061a7526a4077aaa) --- script/analyse/check_migration_quality_tui.py | 23 +++++--- script/analyse/check_migration_residue.py | 5 +- script/todo/migration_status.py | 18 ++++++ script/todo/todo_i18n.py | 8 +++ test/test_check_migration_quality.py | 58 ++++++++++++++++++- test/test_migration_status.py | 35 +++++++++++ 6 files changed, 135 insertions(+), 12 deletions(-) diff --git a/script/analyse/check_migration_quality_tui.py b/script/analyse/check_migration_quality_tui.py index e0fcf57..23a0df0 100644 --- a/script/analyse/check_migration_quality_tui.py +++ b/script/analyse/check_migration_quality_tui.py @@ -160,7 +160,7 @@ def extra_rows(presents, dct=None): # dans son nom — « test_neutralize » ne dit pas 12.0. version = quality.version_of(base, dct) palier = str(version) if version else quality.event_step(event, base) - icone = "❌" if event["status"] else "✅" + icone, _teinte = status.verdict_mark(event["status"]) lst.append( { "kind": "verdict", @@ -349,6 +349,17 @@ def extra_pane(row, colour=False): return "" +# Ce que le chiffre veut dire, pour TOUS les outils — la convention est +# écrite dans todo_upgrade.run_tool. Le sens PROPRE à chaque outil, lui, +# se dit plus bas : « 1 » n'a pas la même gravité pour un test de fumée +# que pour un nettoyage. +SENS_DU_CODE = { + 0: "nothing to report", + 1: "findings", + 2: "the tool failed", +} + + def log_excerpt_lines(row, colour=False): """Le passage du journal d'étape qui entoure ce verdict. @@ -427,12 +438,9 @@ def verdict_pane(row, colour=False): """Ce qu'un verdict dit, ce que son code signifie, et ce qu'on en lit.""" event = row.get("event") or {} rate = bool(event.get("status")) + icone, teinte = status.verdict_mark(event.get("status")) lignes = [ - status.paint( - f"{'❌' if rate else '✅'} {event.get('name')}", - "fail" if rate else "ok", - colour, - ), + status.paint(f"{icone} {event.get('name')}", teinte, colour), "", # Le palier est la version d'ODOO, pas le compteur du pilote : # « 4.1.I » désigne la première étape du quatrième bloc, et la @@ -441,7 +449,8 @@ def verdict_pane(row, colour=False): f" {t('step'):<10} {quality.event_step(event)}" f" ({event.get('step')})", f" {t('when'):<10} {event.get('at')[:19]}", - f" {t('status'):<10} {event.get('status')}", + f" {t('status'):<10} {event.get('status')}" + f" ({t(SENS_DU_CODE.get(event.get('status'), 'the tool failed'))})", "", f" {t('what it ran')}", f" {event.get('detail', '')[:150]}", diff --git a/script/analyse/check_migration_residue.py b/script/analyse/check_migration_residue.py index 0de9d8b..65d0538 100755 --- a/script/analyse/check_migration_residue.py +++ b/script/analyse/check_migration_residue.py @@ -349,10 +349,11 @@ def verdicts_block(database, colour=True, path=None, lignes_avant=6): for event in ratés: version = quality.version_of(quality.event_database(event), dct) palier = str(version) if version else quality.event_step(event) + icone, teinte = status.verdict_mark(event["status"]) lignes.append( paint( - f"❌ {palier.rjust(6)} {event['name']}", - "broken", + f"{icone} {palier.rjust(6)} {event['name']}", + "broken" if teinte == "fail" else "watch", colour, ) ) diff --git a/script/todo/migration_status.py b/script/todo/migration_status.py index d8f2061..c736f2d 100755 --- a/script/todo/migration_status.py +++ b/script/todo/migration_status.py @@ -93,6 +93,24 @@ def paint(texte, couleur, actif=True): VERDICT_COLOUR = {0: "ok", 1: "warn", 2: "fail"} +# La convention des outils de migration, écrite dans todo_upgrade.run_tool : +# « 0 rien à signaler, 1 des trouvailles, 2 l'outil a échoué ». L'icône vit +# à côté de la couleur pour qu'elles ne puissent pas dire deux choses +# différentes — un écran affichait ❌ sur un 1, alors que database_cleanup +# imprime lui-même « This is a warning, not a failure ». +VERDICT_ICON = {0: "✅", 1: "⚠", 2: "❌"} + + +def verdict_mark(statut): + """(icône, couleur) pour ce code de sortie. Inconnu ⇒ traité comme un échec.""" + try: + code = int(statut or 0) + except (TypeError, ValueError): + code = 2 + if code not in VERDICT_ICON: + code = 2 + return VERDICT_ICON[code], VERDICT_COLOUR[code] + def read(path=DEFAULT_PATH): """La progression, complétée par ce qui a été écrit SUR DISQUE. diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index f1a9f1b..5a58e7b 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -6979,6 +6979,14 @@ TRANSLATIONS = { }, # --- Écran de qualité : Verdicts, Validation, Revue --- # --- Journal d'étape et bascule de version --- + "findings": { + "fr": "des trouvailles", + "en": "findings", + }, + "the tool failed": { + "fr": "l'outil a échoué", + "en": "the tool failed", + }, "capture unavailable": { "fr": "capture indisponible", "en": "capture unavailable", diff --git a/test/test_check_migration_quality.py b/test/test_check_migration_quality.py index 7074e5c..0dbe03d 100644 --- a/test/test_check_migration_quality.py +++ b/test/test_check_migration_quality.py @@ -1923,14 +1923,14 @@ class TestEveryVerdictIsListed(Base): for ligne in lignes: self.assertIn("✅", ligne["label"]) - def test_a_failure_and_a_success_sit_side_by_side(self): + def test_a_finding_and_a_success_sit_side_by_side(self): lst = qtui.extra_rows([], self.journal(1, 0)) icones = [ - "❌" if "❌" in r["label"] else "✅" + r["label"].strip().split()[0] for r in lst if r["kind"] == "verdict" ] - self.assertEqual(["❌", "✅"], icones) + self.assertEqual(["⚠", "✅"], icones) def test_the_header_counts_failures_over_the_total(self): lst = qtui.extra_rows([], self.journal(1, 0, 0)) @@ -1979,6 +1979,58 @@ class TestEveryVerdictIsListed(Base): self.assertEqual(1, len(lignes)) +class TestTheScreenObeysTheConvention(Base): + """L'écran peignait en ❌ ce que l'outil appelle un avertissement.""" + + def journal(self, statut): + return { + "lst_event": [ + evenement( + status=statut, + name="database_cleanup", + detail="./script/odoo/migration/database_cleanup.py" + " -d base_upgrade_18", + ) + ], + "config_database_name": "base", + "target_odoo_version": "18.0", + "state_4_upgrade_odoo_lst": [1, 2, 3, 4, 5, 6], + } + + def ligne(self, statut): + lst = qtui.extra_rows([], self.journal(statut)) + return [r for r in lst if r["kind"] == "verdict"][0] + + def test_findings_are_a_warning_in_the_column(self): + # database_cleanup imprime « This is a warning, not a failure » + # avant de rendre 1 : l'écran doit dire la même chose. + self.assertIn("⚠", self.ligne(1)["label"]) + self.assertNotIn("❌", self.ligne(1)["label"]) + + def test_a_tool_that_failed_is_still_red(self): + self.assertIn("❌", self.ligne(2)["label"]) + + def test_nothing_to_report_stays_green(self): + self.assertIn("✅", self.ligne(0)["label"]) + + def test_the_panel_header_agrees_with_the_column(self): + for statut in (0, 1, 2): + ligne = self.ligne(statut) + icone = ligne["label"].strip().split()[0] + self.assertTrue( + qtui.extra_pane(ligne).startswith(icone), + (statut, icone, qtui.extra_pane(ligne)[:20]), + ) + + def test_the_panel_spells_out_what_the_number_means(self): + # « statut 1 » ne dit rien à qui ne connaît pas la convention. + texte = qtui.extra_pane(self.ligne(1)) + self.assertIn(qtui.t("findings"), texte) + + def test_the_meaning_covers_the_whole_convention(self): + self.assertEqual({0, 1, 2}, set(qtui.SENS_DU_CODE)) + + class TestTheStepLogInThePanel(Base): """Ce que le journal contient vraiment, dit sans détour.""" diff --git a/test/test_migration_status.py b/test/test_migration_status.py index cb963e7..1cbcfc1 100644 --- a/test/test_migration_status.py +++ b/test/test_migration_status.py @@ -1548,6 +1548,41 @@ class TestNothingOnThatScreenCanHang(Base): self.assertEqual(TodoUpgrade.ask_ui(), "tui") +class TestWhatTheExitCodeMeans(Base): + """0 rien à signaler, 1 des trouvailles, 2 l'outil a échoué. + + La convention est écrite dans todo_upgrade.run_tool, et elle n'est pas + décorative : `database_cleanup` imprime lui-même « This is a warning, + not a failure » avant de rendre 1. Un écran qui peint ce 1 en rouge + contredit l'outil, et l'on finit par ignorer les deux. + """ + + def test_zero_is_green(self): + self.assertEqual(("✅", "ok"), status.verdict_mark(0)) + + def test_one_is_a_finding_not_a_failure(self): + icone, teinte = status.verdict_mark(1) + self.assertEqual("⚠", icone) + self.assertEqual("warn", teinte) + self.assertNotEqual("fail", teinte) + + def test_two_is_the_tool_itself_failing(self): + self.assertEqual(("❌", "fail"), status.verdict_mark(2)) + + def test_an_unknown_code_is_treated_as_a_failure(self): + # Un code qu'on ne sait pas lire ne doit pas passer pour un succès. + for inconnu in (3, 127, -1, "oui", None, ""): + icone, teinte = status.verdict_mark(inconnu) + if inconnu in (None, ""): + self.assertEqual("ok", teinte, inconnu) + else: + self.assertEqual("fail", teinte, inconnu) + + def test_the_icon_and_the_colour_cannot_drift(self): + # Elles vivent au même endroit précisément pour cela. + self.assertEqual(set(status.VERDICT_ICON), set(status.VERDICT_COLOUR)) + + class TestWhatGetsRecorded(Base): def upgrade(self): obj = TodoUpgrade.__new__(TodoUpgrade)