[FIX] verdicts de migration : distinguer trouvaille et échec d'outil

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)
This commit is contained in:
Mathieu Benoit 2026-08-28 01:09:24 -04:00
parent b7d68cd199
commit 809273eb34
6 changed files with 135 additions and 12 deletions

View file

@ -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]}",

View file

@ -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,
)
)

View file

@ -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.

View file

@ -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",

View file

@ -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."""

View file

@ -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)