From 1d891e6e33eec7f05e3bf3724a38c07d482f3cfd Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Mon, 14 Sep 2026 14:41:19 -0400 Subject: [PATCH] =?UTF-8?q?[IMP]=20tests=20longs=20:=20confirmer=20avant?= =?UTF-8?q?=20de=20cr=C3=A9er=20ou=20de=20d=C3=A9truire=20des=20machines?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The long-test menu printed the command and ran it at once: a mistyped digit created real VMs, and « --detruire » removes machines with their disks. The command stays printed, since it is what makes the question answerable, and the prompt follows it. Warning and question are chosen from the arguments, so a destruction is never confirmed with the words of a launch. A dry run or a report creates nothing and asks nothing. Covered by test/test_longtest_confirm.py. --- FR --- Le menu des tests longs affichait la commande et l'exécutait aussitôt : un chiffre tapé de travers créait de vraies VM, et « --detruire » efface des machines avec leurs disques. La commande reste affichée, c'est elle qui rend la question répondable, et l'invite la suit. L'avertissement et la question se choisissent sur les arguments : une destruction n'est jamais confirmée avec les mots d'un lancement. Un plan à blanc ou un rapport ne crée rien et ne demande rien. Couvert par test/test_longtest_confirm.py. Assisted-by: Claude Opus 5 --- script/todo/longtest_menu.py | 45 +++++++++- script/todo/todo_i18n.py | 16 ++++ test/test_longtest_confirm.py | 151 ++++++++++++++++++++++++++++++++++ 3 files changed, 209 insertions(+), 3 deletions(-) create mode 100644 test/test_longtest_confirm.py diff --git a/script/todo/longtest_menu.py b/script/todo/longtest_menu.py index 39076e9..04bcf91 100644 --- a/script/todo/longtest_menu.py +++ b/script/todo/longtest_menu.py @@ -30,12 +30,38 @@ class LongTestMenuMixin: chemin = os.path.join(os.getcwd(), LONGTEST_DIR, nom) return chemin if os.path.exists(chemin) else "" - def _longtest_run(self, nom, args=""): + # Ce qui ne crée aucune machine : un plan, un rapport, une liste. Ces + # commandes-là ne méritent pas de question — une invite qu'on apprend à + # confirmer sans lire ne protège plus rien le jour où elle compte. + _LONGTEST_SANS_EFFET = ("--dry-run", "--rapport") + + @staticmethod + def _longtest_question(args): + """L'avertissement et la question qui vont avec ces arguments. + + Rend un couple de CLÉS de traduction, jamais du texte : l'invite est + bilingue comme le reste du menu. + """ + if "--detruire" in (args or ""): + return ( + "This destroys the machines of this test and their disks.", + "Destroy the machines of this test?", + ) + return ( + "This creates real VMs and takes a while.", + "Run this long test?", + ) + + def _longtest_run(self, nom, args="", demander=None): """Lance un test long, sortie en DIRECT. En direct parce qu'il dure des heures : capturer sa sortie pour l'afficher à la fin, c'est ne rien montrer pendant tout ce temps — et c'est justement la progression étage par étage qui intéresse. + + `demander` : None laisse la commande décider — on confirme dès qu'elle + peut créer de vraies machines. Un appelant qui a DÉJÀ posé sa question + passe False, sans quoi l'opérateur répondrait deux fois à la même. """ chemin = self._longtest_script(nom) if not chemin: @@ -45,6 +71,19 @@ class LongTestMenuMixin: if args: cmd += f" {args}" print(f"\n{t('Will execute:')} {cmd}") + if demander is None: + demander = not any( + d in (args or "") for d in self._LONGTEST_SANS_EFFET + ) + if demander: + # Une frappe ne doit suffire ni à créer de vraies machines, ni à + # en effacer. La question doit dire LAQUELLE des deux on fait : + # confirmer « lancer ce test long » devant une destruction fait + # répondre oui à autre chose que ce qui va arriver. + avertissement, question = self._longtest_question(args) + print(f" {t(avertissement)}") + if not click.confirm(t(question)): + return self.execute.exec_command_live(cmd, source_erplibre=False) def prompt_execute_longtest(self): @@ -123,9 +162,9 @@ class LongTestMenuMixin: mène pas directement à un « qm destroy --purge ». """ for script in ("deep_proxmox.py", "deep_qemu.py", "qemu_cache.py"): - self._longtest_run(script, "--detruire --dry-run") + self._longtest_run(script, "--detruire --dry-run", demander=False) if self._is_yes(input(f"\n{t('Destroy all that? (y/N): ')}")): - self._longtest_run(script, "--detruire") + self._longtest_run(script, "--detruire", demander=False) def _longtest_depart(self, script): """D'où part la descente : une VM neuve, ou un hôte qu'on a déjà. diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index d4ed31b..dbc9fa0 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -1563,6 +1563,14 @@ TRANSLATIONS = { "fr": "Les retirer depuis l'entrée 4 de ce menu.", "en": "Remove them from entry 4 of this menu.", }, + "This destroys the machines of this test and their disks.": { + "fr": "Cela détruit les machines de ce test et leurs disques.", + "en": "This destroys the machines of this test and their disks.", + }, + "Destroy the machines of this test?": { + "fr": "Détruire les machines de ce test ?", + "en": "Destroy the machines of this test?", + }, "Machines created:": { "fr": "Machines créées :", "en": "Machines created:", @@ -2056,6 +2064,14 @@ TRANSLATIONS = { "fr": "Le témoin mesure ce que coûte l'ABSENCE de cache.", "en": "The control run measures what NOT caching costs.", }, + "This creates real VMs and takes a while.": { + "fr": "Cela crée de vraies VM et prend du temps.", + "en": "This creates real VMs and takes a while.", + }, + "Run this long test?": { + "fr": "Lancer ce test long ?", + "en": "Run this long test?", + }, "Install the download cache shared by the QEMU VMs of this host": { "fr": "Installer le cache de téléchargement partagé par les VM QEMU de cet hôte", "en": "Install the download cache shared by the QEMU VMs of this host", diff --git a/test/test_longtest_confirm.py b/test/test_longtest_confirm.py new file mode 100644 index 0000000..94599a5 --- /dev/null +++ b/test/test_longtest_confirm.py @@ -0,0 +1,151 @@ +#!/usr/bin/env python3 +# © 2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""Un test long ne se lance pas sur une seule frappe. + +Ces scripts créent de vraies machines et durent. Le menu affichait la +commande puis l'exécutait aussitôt : un chiffre tapé de travers partait donc +créer trois VM, et il n'y avait plus qu'à attendre pour les détruire. + +La question n'est pas posée pour tout : un plan à blanc ou un rapport ne crée +rien, et une invite qu'on apprend à confirmer sans lire ne protège plus rien +le jour où elle compte. Le partage se fait sur les arguments, et ce test le +vérifie dans les deux sens. +""" + +import sys +import unittest +from pathlib import Path +from unittest import mock + +RACINE = Path(__file__).resolve().parent.parent +sys.path.insert(0, str(RACINE)) + +from script.todo import todo_i18n # noqa: E402 +from script.todo.todo import TODO # noqa: E402 + + +class FauxExecute: + """Retient ce qu'on lui demande de lancer, sans rien lancer.""" + + def __init__(self): + self.commandes = [] + + def exec_command_live(self, cmd, **_kw): + self.commandes.append(cmd) + + +def menu(reponse=True): + todo = TODO.__new__(TODO) + todo.execute = FauxExecute() + return todo, mock.patch( + "script.todo.longtest_menu.click.confirm", return_value=reponse + ) + + +class TestConfirmationDesTestsLongs(unittest.TestCase): + def test_une_vraie_execution_demande(self): + todo, patch = menu(reponse=True) + with patch as confirm: + todo._longtest_run("qemu_cache.py", "") + self.assertTrue(confirm.called, "aucune confirmation demandée") + self.assertEqual(len(todo.execute.commandes), 1) + + def test_un_refus_ne_lance_rien(self): + todo, patch = menu(reponse=False) + with patch: + todo._longtest_run("qemu_cache.py", "") + self.assertEqual( + todo.execute.commandes, [], "le test a démarré malgré le refus" + ) + + def test_le_plan_a_blanc_ne_demande_pas(self): + """Il ne crée rien : demander l'aurait rendue machinale.""" + todo, patch = menu() + with patch as confirm: + todo._longtest_run("qemu_cache.py", "--dry-run") + self.assertFalse(confirm.called) + self.assertEqual(len(todo.execute.commandes), 1) + + def test_le_rapport_ne_demande_pas(self): + todo, patch = menu() + with patch as confirm: + todo._longtest_run("qemu_cache.py", "--rapport") + self.assertFalse(confirm.called) + self.assertEqual(len(todo.execute.commandes), 1) + + def test_un_appelant_peut_couper_la_question(self): + """La destruction pose déjà la sienne : la doubler ferait répondre + deux fois à la même chose.""" + todo, patch = menu() + with patch as confirm: + todo._longtest_run("qemu_cache.py", "--detruire", demander=False) + self.assertFalse(confirm.called) + self.assertEqual(len(todo.execute.commandes), 1) + + def test_la_question_dit_ce_qui_va_arriver(self): + """Détruire n'est pas lancer. + + L'invite était la même pour les deux : « Cela crée de vraies VM… + Lancer ce test long ? » s'affichait devant « --detruire », qui efface + des machines et leurs disques. On répondait oui à autre chose que ce + qui allait arriver, et c'est l'acte le moins rattrapable des deux. + """ + todo, patch = menu() + with patch as confirm: + todo._longtest_run("qemu_cache.py", "--detruire") + pose = confirm.call_args.args[0] + self.assertIn("Détruire", pose, f"question posée : {pose}") + self.assertNotIn("Lancer", pose) + + def test_une_creation_pose_toujours_la_sienne(self): + todo, patch = menu() + with patch as confirm: + todo._longtest_run("qemu_cache.py", "--sans-cache") + pose = confirm.call_args.args[0] + self.assertIn("Lancer", pose, f"question posée : {pose}") + + def test_les_deux_avertissements_different(self): + """L'un annonce une création, l'autre un effacement : les confondre + est ce qui rend une confirmation machinale.""" + from script.todo.longtest_menu import LongTestMenuMixin as L + + creer = L._longtest_question("") + defaire = L._longtest_question("--detruire") + self.assertNotEqual(creer, defaire) + for cle in creer + defaire: + self.assertIn( + cle, + todo_i18n.TRANSLATIONS, + f"« {cle} » n'est pas une clé de traduction", + ) + + def test_la_commande_reste_affichee(self): + """Elle l'était déjà, et c'est ce qui rend la question répondable.""" + todo, patch = menu() + with patch, mock.patch("builtins.print") as ecrit: + todo._longtest_run("qemu_cache.py", "--dry-run") + dit = " ".join(str(a) for c in ecrit.call_args_list for a in c.args) + self.assertIn("qemu_cache.py --dry-run", dit) + + def test_la_destruction_ne_demande_pas_deux_fois(self): + """Les deux appels de _longtest_defaire portent demander=False.""" + src = (RACINE / "script" / "todo" / "longtest_menu.py").read_text( + encoding="utf-8" + ) + bloc = src[ + src.index("def _longtest_defaire") : src.index( + "def _longtest_depart" + ) + ] + for appel in ("--detruire --dry-run", '"--detruire"'): + self.assertIn( + "demander=False", + bloc, + f"l'appel {appel} de la destruction pose une seconde question", + ) + + +if __name__ == "__main__": + unittest.main()