From 24f38c79ad13db870d773be7a54603d664db157d Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sun, 23 Aug 2026 02:02:37 -0400 Subject: [PATCH] =?UTF-8?q?[FIX]=20migration:=20un=20OpenUpgrade=20rat?= =?UTF-8?q?=C3=A9=20ne=20doit=20pas=20passer=20pour=20fait?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lst_upgrade_odoo` n'est pas une copie : `dct_progression.get()` rend l'objet stocké. La commande y était inscrite AVANT de tourner, donc le premier `write_config()` la gravait — y compris celui du chemin d'échec, qui remet pourtant le drapeau de clonage à zéro pour forcer un nouvel essai. La reprise sautait alors OpenUpgrade. Mesuré sur test_neutralize_upgrade_18, arrêté sur l'erreur des thèmes : base = 17.0.1.3, clone à refaire, et sa commande de migration 18 déjà consignée. Relancer aurait laissé une base 17 sous le code 18. On l'inscrit après la réussite, là où le commentaire la situait déjà. --- EN --- `lst_upgrade_odoo` is not a copy: `dct_progression.get()` returns the stored object. The command was recorded BEFORE it ran, so the first `write_config()` persisted it — including the one on the failure path, which resets the clone flag precisely to force a fresh attempt. A resume then skipped OpenUpgrade. Measured on test_neutralize_upgrade_18, halted on the theme error: base = 17.0.1.3, clone pending, and its 18 migration command already recorded. Resuming would have left a 17 database under 18 code. We record it after success, where the comment already placed it. Assisted-by: Claude Opus 5 --- script/todo/todo_upgrade.py | 11 +++- test/test_uninstall_verified.py | 93 +++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-) diff --git a/script/todo/todo_upgrade.py b/script/todo/todo_upgrade.py index c644526..07e633b 100755 --- a/script/todo/todo_upgrade.py +++ b/script/todo/todo_upgrade.py @@ -2615,7 +2615,15 @@ class TodoUpgrade: cmd_upgrade = f".venv.{erplibre_version}/bin/python ./odoo{next_version}.0/OCA_OpenUpgrade/odoo-bin -c ./config.conf --update all --no-http --stop-after-init -d {database_name_upgrade}" else: cmd_upgrade = f"./run.sh --upgrade-path=./odoo{next_version}.0/OCA_OpenUpgrade/openupgrade_scripts/scripts --update all -c config.conf --stop-after-init --no-http --load=base,web,openupgrade_framework -d {database_name_upgrade}" - lst_upgrade_odoo[index] = cmd_upgrade + # NE PAS enregistrer la commande ici. `lst_upgrade_odoo` + # EST la liste de `dct_progression` — `.get` rend l'objet, + # pas une copie — donc la muter maintenant la fait persister + # au premier write_config() venu, y compris celui du chemin + # d'échec juste en dessous. L'étape passait alors pour faite + # et la reprise SAUTAIT OpenUpgrade : mesuré sur + # test_neutralize_upgrade_18, resté en base 17.0.1.3 avec sa + # commande 18 déjà consignée. On l'enregistre après la + # réussite, où le commentaire dit déjà qu'elle appartient. # Record the website COW views before the data migration. The # upgrade silently deletes and recreates copies (measured on @@ -2682,6 +2690,7 @@ class TodoUpgrade: f"after_{next_version}", ) + lst_upgrade_odoo[index] = cmd_upgrade self.dct_progression["state_4_upgrade_odoo_lst"] = ( lst_upgrade_odoo ) diff --git a/test/test_uninstall_verified.py b/test/test_uninstall_verified.py index a38b8f8..a1ccc35 100644 --- a/test/test_uninstall_verified.py +++ b/test/test_uninstall_verified.py @@ -185,5 +185,98 @@ class TestNoStepWritesAnotherStepsFlag(unittest.TestCase): ) +class TestAFailedOpenUpgradeStaysUnrecorded(unittest.TestCase): + """Un OpenUpgrade raté ne doit pas passer pour fait. + + `lst_upgrade_odoo` n'est pas une copie : `dct_progression.get()` rend + l'objet stocké. L'affecter avant l'exécution le faisait persister au + premier `write_config()` venu — celui du chemin d'échec compris, qui + remet pourtant le drapeau de clonage à zéro pour forcer un nouvel + essai. La reprise sautait alors OpenUpgrade et laissait une base 17 + tourner sous le code 18. Mesuré sur test_neutralize_upgrade_18 : + base = 17.0.1.3, et sa commande de migration déjà consignée. + + Conduire `execute_odoo_upgrade` en vrai demanderait une migration + complète ; la faute est un ORDRE dans le source, et c'est l'ordre + qu'on mesure. + """ + + def arbre(self): + with io.open(SOURCE, encoding="utf-8") as handle: + return ast.parse(handle.read()) + + def lignes_affectation(self): + lignes = [] + for noeud in ast.walk(self.arbre()): + if not isinstance(noeud, ast.Assign): + continue + for cible in noeud.targets: + if ( + isinstance(cible, ast.Subscript) + and isinstance(cible.value, ast.Name) + and cible.value.id == "lst_upgrade_odoo" + ): + lignes.append(noeud.lineno) + return lignes + + @staticmethod + def _remet_le_clone_a_zero(noeud): + """Ce bloc renonce-t-il en redemandant un clonage neuf ? + + Le repère est l'affectation `lst_clone_odoo[index] = False` : c'est + elle qui distingue « je renonce, refais le clone » de l'étape de + clonage elle-même, qui écrit `= True` et vit ailleurs. Chercher les + seuls NOMS attrapait les deux, et l'ancre tombait 700 lignes trop + haut — le test passait alors sur n'importe quel ordre. + """ + for petit in ast.walk(noeud): + if not isinstance(petit, ast.Assign): + continue + if not ( + isinstance(petit.value, ast.Constant) + and petit.value.value is False + ): + continue + for cible in petit.targets: + if ( + isinstance(cible, ast.Subscript) + and isinstance(cible.value, ast.Name) + and cible.value.id == "lst_clone_odoo" + ): + return True + return False + + def ligne_abandon(self): + """Le `return` qui renonce après un OpenUpgrade raté.""" + lignes = [ + max(n.lineno for n in ast.walk(noeud) if isinstance(n, ast.Return)) + for noeud in ast.walk(self.arbre()) + if isinstance(noeud, ast.If) + and self._remet_le_clone_a_zero(noeud) + and any(isinstance(n, ast.Return) for n in ast.walk(noeud)) + ] + return min(lignes) if lignes else None + + def test_both_anchors_are_found(self): + # Sans cette borne, les tests suivants passeraient à vide le jour + # où l'une des deux formes change. + self.assertTrue(self.lignes_affectation()) + self.assertIsNotNone(self.ligne_abandon()) + + def test_the_step_is_recorded_only_after_the_failure_path_gave_up(self): + abandon = self.ligne_abandon() + for ligne in self.lignes_affectation(): + self.assertGreater( + ligne, + abandon, + "lst_upgrade_odoo est marqué fait avant que l'échec ait" + " pu renoncer : la reprise sautera OpenUpgrade", + ) + + def test_it_is_recorded_exactly_once(self): + # Deux affectations, et l'une repasserait devant l'échec. + self.assertEqual(len(self.lignes_affectation()), 1) + + if __name__ == "__main__": unittest.main()