diff --git a/script/todo/migration_status.py b/script/todo/migration_status.py index 579e10c..bba12b9 100755 --- a/script/todo/migration_status.py +++ b/script/todo/migration_status.py @@ -242,22 +242,39 @@ def verdict(status): def tests_summary(dct): - """Le DERNIER verdict de chaque outil, et le compte des passages. + """Le dernier verdict de chaque outil, PAR ÉTAPE, et son nombre de passages. - Un outil relancé après correction a deux verdicts contradictoires dans - le journal, et c'est le dernier qui décrit la base telle qu'elle est. - Afficher les deux sans les distinguer ferait lire une réparation comme - un échec persistant. + Par étape, car c'est la question qu'on pose. Une migration lance le + test de fumée à CHAQUE palier ; regrouper sur le seul nom d'outil n'en + laissait qu'une ligne, et l'on lisait « smoke_public_url ✅ » sans voir + que le palier 14 était passé et le 17 tombé. + + Dans une étape, le DERNIER verdict l'emporte : un outil relancé après + correction a deux verdicts contradictoires, et c'est le second qui + décrit la base telle qu'elle est. Les afficher tous deux sans les + distinguer ferait lire une réparation comme un échec persistant. + + L'ordre est celui du journal, donc celui de la migration. Trier les + étapes par leur nom mettrait « 4.10 » avant « 4.2 ». """ dernier = {} for item in events(dct, kind="test"): - nom = item.get("name") or "?" - entree = dernier.setdefault(nom, {"name": nom, "runs": 0}) + cle = (item.get("step") or "", item.get("name") or "?") + entree = dernier.setdefault( + cle, {"name": cle[1], "step": cle[0], "runs": 0} + ) entree["runs"] += 1 entree["status"] = item.get("status") entree["at"] = item.get("at") - entree["step"] = item.get("step") - return [dernier[nom] for nom in sorted(dernier)] + return list(dernier.values()) + + +def tests_by_step(dct): + """Les verdicts groupés sous leur étape, dans l'ordre de la migration.""" + par_etape = {} + for item in tests_summary(dct): + par_etape.setdefault(item["step"], []).append(item) + return list(par_etape.items()) def failures(dct): @@ -325,18 +342,22 @@ def render_text(dct, limit_cmd=12, colour=None): lignes.append(f"\n🧪 {t('Test results')}") if not lst_test: lignes.append(f" {t('No tool has run yet.')}") - for item in lst_test: - icone, phrase = verdict(item.get("status")) - rejeu = ( - f" ({item['runs']} {t('runs')})" - if item.get("runs", 1) > 1 - else "" - ) - teinte = VERDICT_COLOUR.get(item.get("status"), "dim") - nom = f"{item['name']:<24}" + for etape, lst_item in tests_by_step(dct): lignes.append( - f" {icone} {paint(nom, teinte, colour)} {phrase}{rejeu}" + f" {paint(etape or t('before the first step'), 'step', colour)}" ) + for item in lst_item: + icone, phrase = verdict(item.get("status")) + rejeu = ( + f" ({item['runs']} {t('runs')})" + if item.get("runs", 1) > 1 + else "" + ) + teinte = VERDICT_COLOUR.get(item.get("status"), "dim") + nom = f"{item['name']:<24}" + lignes.append( + f" {icone} {paint(nom, teinte, colour)} {phrase}{rejeu}" + ) lst_failure = failures(dct) lignes.append(f"\n❌ {t('Commands that failed')} : {len(lst_failure)}") diff --git a/script/todo/migration_status_tui.py b/script/todo/migration_status_tui.py index 93ec246..865eb7d 100644 --- a/script/todo/migration_status_tui.py +++ b/script/todo/migration_status_tui.py @@ -64,10 +64,15 @@ def rows(dct): lst = [] for item in status.tests_summary(dct): icone, _phrase = status.verdict(item.get("status")) + # Le NUMÉRO de l'étape en tête : une migration lance le même outil + # à chaque palier, et sans lui la liste montrait six lignes + # identiques sans dire laquelle appartenait à quel palier. + numero = (item.get("step") or "").split(" - ")[0] + tete = f"{numero} " if numero else "" lst.append( { "kind": "test", - "label": f"{icone} {item['name']}", + "label": f"{icone} {tete}{item['name']}", "detail": str(item.get("runs", 1)), "data": item, } @@ -99,9 +104,9 @@ def pane_text(dct, row, colour=False): teinte = status.VERDICT_COLOUR.get(item.get("status"), "dim") lignes = [ f"{icone} {status.paint(item['name'], teinte, colour)}", + f" {status.paint(item.get('step') or '?', 'step', colour)}", f" {phrase} ({t('exit code')} {item.get('status')})", f" {t('runs')} : {item.get('runs')}", - f" {t('current step')} : {item.get('step') or '?'}", f" {item.get('at') or ''}", ] return "\n".join(lignes) diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index 96b4c42..d0e98d3 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -4044,7 +4044,20 @@ class TodoUpgrade: return status def add_comment_progression(self, comment): + """Marquer une étape dans le journal, et devenir CETTE étape. + + Vingt en-têtes d'étape passent par ici contre sept par + `print_step` : toute la boucle des paliers n'utilise que celle-ci. + Ne poser l'étape courante que dans l'autre laissait donc chaque + verdict d'outil estampillé d'une étape périmée — et le résumé des + tests semblait global alors qu'il aurait dû suivre les paliers. + + C'est le MÊME marqueur que `journal_by_step` découpe : une seule + notion d'étape, posée à un seul endroit. + """ comment_to_add = f"# {comment}" self.lst_command_executed.append(comment_to_add) self.dct_progression["command_executed"] = self.lst_command_executed + self.current_step = comment + self.open_step_log(comment) self.write_config() diff --git a/test/test_migration_status.py b/test/test_migration_status.py index 9b98fb8..f7b9426 100644 --- a/test/test_migration_status.py +++ b/test/test_migration_status.py @@ -123,6 +123,120 @@ class TestARepairMustNotReadAsAFailure(Base): smoke = [x for x in lst if x["name"] == "smoke_public_url"][0] self.assertEqual(smoke["status"], 0) + def test_the_verdicts_are_kept_PER_STEP(self): + """La demande, et le défaut qu'elle a révélé. + + Une migration lance le test de fumée à CHAQUE palier. Regrouper sur + le seul nom d'outil n'en laissait qu'une ligne : on lisait + « smoke_public_url ✅ » sans voir que le palier 14 était passé et + le 17 tombé. + """ + dct = progression( + lst_event=[ + { + "at": "1", + "step": "4.1 - v14", + "kind": "test", + "name": "smoke_public_url", + "status": 0, + }, + { + "at": "2", + "step": "4.2 - v15", + "kind": "test", + "name": "smoke_public_url", + "status": 2, + }, + ] + ) + lst = status.tests_summary(dct) + self.assertEqual(len(lst), 2) + self.assertEqual( + [(x["step"], x["status"]) for x in lst], + [("4.1 - v14", 0), ("4.2 - v15", 2)], + ) + + def test_a_repair_within_a_step_still_collapses(self): + # Deux verdicts pour le MÊME palier : c'est une réparation, pas + # deux paliers. Le second décrit la base telle qu'elle est. + dct = progression( + lst_event=[ + { + "at": "1", + "step": "4.2 - v15", + "kind": "test", + "name": "smoke_public_url", + "status": 2, + }, + { + "at": "2", + "step": "4.2 - v15", + "kind": "test", + "name": "smoke_public_url", + "status": 0, + }, + ] + ) + lst = status.tests_summary(dct) + self.assertEqual(len(lst), 1) + self.assertEqual(lst[0]["status"], 0) + self.assertEqual(lst[0]["runs"], 2) + + def test_the_order_is_the_migration_s_own(self): + # Trier les étapes par leur nom mettrait « 4.10 » avant « 4.2 ». + dct = progression( + lst_event=[ + { + "at": "1", + "step": "4.2 - v15", + "kind": "test", + "name": "outil", + "status": 0, + }, + { + "at": "2", + "step": "4.10 - v18", + "kind": "test", + "name": "outil", + "status": 0, + }, + ] + ) + self.assertEqual( + [etape for etape, _lst in status.tests_by_step(dct)], + ["4.2 - v15", "4.10 - v18"], + ) + + def test_the_report_shows_the_step_number(self): + dct = progression( + lst_event=[ + { + "at": "1", + "step": "4.1 - Ready to work with version 14", + "kind": "test", + "name": "database_cleanup", + "status": 0, + }, + ] + ) + texte = status.render_text(dct, colour=False) + self.assertIn("4.1 - Ready to work with version 14", texte) + + def test_the_full_screen_shows_it_too(self): + dct = progression( + lst_event=[ + { + "at": "1", + "step": "4.1 - v14", + "kind": "test", + "name": "smoke_public_url", + "status": 0, + }, + ] + ) + ligne = [x for x in tui.rows(dct) if x["kind"] == "test"][0] + self.assertIn("4.1", ligne["label"]) + def test_but_the_earlier_runs_are_still_counted(self): # Les taire ferait croire à un premier essai réussi, et l'on # perdrait la trace de ce qui a demandé une réparation. @@ -439,6 +553,46 @@ class TestWhatSurvivesClosingTheTool(DiskCase): self.assertIn("premier", noms) self.assertIn("dernier", noms) + def test_a_step_header_becomes_the_current_step(self): + """La cause racine de « ça semble global ». + + Vingt en-têtes d'étape passent par `add_comment_progression` contre + sept par `print_step`, et toute la boucle des paliers n'utilise que + la première. Ne poser l'étape courante que dans l'autre laissait + chaque verdict estampillé d'une étape périmée. + """ + obj = self.upgrade() + obj.add_comment_progression("4.2 - Ready to work with version 15") + obj.record_event("test", "smoke_public_url", 0) + obj.close_step_log() + lst = status.merge_events({"config_database_name": "essai_db"}) + self.assertEqual(lst[0]["step"], "4.2 - Ready to work with version 15") + + def test_a_step_header_opens_its_log_too(self): + # Même raison : sans cela, la sortie des commandes d'un palier + # allait dans le fichier de l'étape d'AVANT. + obj = self.upgrade() + obj.add_comment_progression("4.2 - Migrate database") + obj.note_step_log("quelque chose") + obj.close_step_log() + self.assertIsNotNone( + status.step_log_path( + {"config_database_name": "essai_db"}, "4.2 - Migrate database" + ) + ) + + def test_two_bumps_do_not_share_a_step(self): + # Le symptôme exact : six paliers, un seul nom d'étape. + obj = self.upgrade() + for palier in ("4.1 - version 14", "4.2 - version 15"): + obj.add_comment_progression(palier) + obj.record_event("test", "smoke_public_url", 0) + obj.close_step_log() + lst = status.merge_events({"config_database_name": "essai_db"}) + self.assertEqual( + [x["step"] for x in lst], ["4.1 - version 14", "4.2 - version 15"] + ) + def test_a_migration_without_a_database_writes_nowhere(self): obj = self.upgrade(database=None) obj.dct_progression = {}