From d35d5592737c52e0b293e15523d2a1ba93f49733 Mon Sep 17 00:00:00 2001 From: Mathieu Benoit Date: Sat, 22 Aug 2026 04:07:19 -0400 Subject: [PATCH] [FIX] smoke: two sites still unpacked three fields from the failure tuple MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own regression. The tuple went from three fields to four and I missed recheck_after_reset, so a live migration died on "too many values to unpack" -- AFTER the COW copy had been reset, at the very moment the tool was about to prove the fix worked. Both sites wanted only the URL, so they index instead of unpacking. That survives the next field; a comment saying "always four elements" does not. The pass had no test at all, which is why the suite and the mutations both stayed green. It has three now, plus a guard that fails on any three-field unpack of that list. --- FR --- Ma régression. Le tuple est passé de trois à quatre champs et j'ai manqué recheck_after_reset : une migration en cours est morte sur « too many values to unpack » — APRÈS la réinitialisation de la copie COW, au moment précis où l'outil allait prouver que le correctif tenait. Les deux sites ne voulaient que l'URL : ils indexent au lieu de dépaqueter. Cela survit au prochain champ ; un commentaire « toujours quatre éléments » non. Cette passe n'avait aucun test, d'où le vert de la suite et des mutations. Elle en a trois, plus un garde qui tombe sur tout dépaquetage à trois champs de cette liste. Assisted-by: Claude Opus 5 --- script/odoo/migration/smoke_public_url.py | 4 +- test/test_smoke_final_url.py | 126 ++++++++++++++++++++++ 2 files changed, 128 insertions(+), 2 deletions(-) diff --git a/script/odoo/migration/smoke_public_url.py b/script/odoo/migration/smoke_public_url.py index 37eccf8..dd80601 100755 --- a/script/odoo/migration/smoke_public_url.py +++ b/script/odoo/migration/smoke_public_url.py @@ -777,7 +777,7 @@ def recheck_after_reset( f"{t('The server never answered on')} {base_url}" ) lst_again = check_urls( - [url for url, _s, _p in lst_failure], timeout=timeout + [echec[0] for echec in lst_failure], timeout=timeout ) if internal_needs_retry(internal_report): reprise = internal_phase( @@ -936,7 +936,7 @@ def main(argv=None): f"\n↻ {t('Re-checked the')} {len(lst_failure)}" f" {t('failing URL(s) after the reset')} :" ) - print(render([url for url, _s, _p in lst_failure], lst_again, None)) + print(render([echec[0] for echec in lst_failure], lst_again, None)) return 1 if (lst_again or internal_failed) else 0 diff --git a/test/test_smoke_final_url.py b/test/test_smoke_final_url.py index 85a82dc..6913b34 100644 --- a/test/test_smoke_final_url.py +++ b/test/test_smoke_final_url.py @@ -150,6 +150,132 @@ class TestTheReport(unittest.TestCase): self.assertNotIn("[500]", texte) +class TestNothingUnpacksTheFailureTupleBlindly(unittest.TestCase): + """Ajouter un champ au tuple d'échec a cassé une migration en cours. + + Le tuple est passé de trois à quatre éléments et deux sites + dépaquetaient encore trois — `too many values to unpack`, en plein + milieu, APRÈS la réinitialisation d'une copie COW. Le commentaire + « TOUJOURS quatre éléments » ne protège de rien : il faut ne pas + dépaqueter quand on ne veut qu'un champ. + """ + + CHEMIN = os.path.join( + os.path.dirname(__file__), + "..", + "script", + "odoo", + "migration", + "smoke_public_url.py", + ) + + def source(self): + with io.open(self.CHEMIN, encoding="utf-8") as handle: + return handle.read() + + def test_no_three_element_unpack_survives(self): + import re + + motif = re.compile( + r"for\s+[a-z_]+,\s*[a-z_]+,\s*[a-z_]+\s+in\s+lst_failure" + ) + trouves = motif.findall(self.source()) + self.assertEqual(trouves, [], f"dépaquetage à trois : {trouves}") + + def test_taking_only_the_url_uses_an_index(self): + # Indexer survit au prochain champ ajouté ; dépaqueter non. + self.assertIn("[echec[0] for echec in lst_failure]", self.source()) + + +class TestRecheckingAfterAReset(unittest.TestCase): + """La passe qui a cassé, exercée pour de vrai. + + Elle ne tournait sous aucun test : c'est pourquoi le dépaquetage à + trois y a survécu à la suite complète, aux mutations, et n'est + tombé qu'en production. + """ + + def setUp(self): + self.vrais = { + nom: getattr(smoke, nom) + for nom in ( + "start_server", + "wait_ready", + "check_urls", + "internal_needs_retry", + "stop_server", + ) + if hasattr(smoke, nom) + } + self.vus = [] + + class FauxServeur: + def __init__(self): + self.arrete = False + + smoke.start_server = lambda *a, **k: FauxServeur() + smoke.wait_ready = lambda *a, **k: True + smoke.internal_needs_retry = lambda rapport: False + if hasattr(smoke, "stop_server"): + smoke.stop_server = lambda *a, **k: None + + def faux_check(lst_url, timeout=30): + self.vus.append(list(lst_url)) + return [] + + smoke.check_urls = faux_check + + def tearDown(self): + for nom, valeur in self.vrais.items(): + setattr(smoke, nom, valeur) + + def test_it_rechecks_exactly_the_urls_that_had_failed(self): + echecs = [ + ("http://h/contactus", 500, ["2837"], "http://h/en/contactus"), + ("http://h/blog", 500, [], "http://h/blog"), + ] + smoke.recheck_after_reset( + "db", + 8169, + "./config.conf", + "http://h", + None, + echecs, + {"failures": []}, + internal=False, + ) + self.assertEqual(self.vus, [["http://h/contactus", "http://h/blog"]]) + + def test_it_rechecks_the_REQUESTED_url_not_the_final_one(self): + # On revérifie ce que le sitemap publie : c'est cette adresse-là + # que les visiteurs demandent. + echecs = [("http://h/a", 500, [], "http://h/z")] + smoke.recheck_after_reset( + "db", + 8169, + "./config.conf", + "http://h", + None, + echecs, + {"failures": []}, + internal=False, + ) + self.assertEqual(self.vus, [["http://h/a"]]) + + def test_an_empty_failure_list_rechecks_nothing(self): + smoke.recheck_after_reset( + "db", + 8169, + "./config.conf", + "http://h", + None, + [], + {"failures": []}, + internal=False, + ) + self.assertEqual(self.vus, [[]]) + + class TestTheLogSurvives(unittest.TestCase): def test_the_previous_run_is_kept(self): # Le journal était ouvert en « w » : relancer le test effaçait la