[FIX] smoke: two sites still unpacked three fields from the failure tuple

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
This commit is contained in:
Mathieu Benoit 2026-08-22 04:07:19 -04:00
parent 437ac4b457
commit d35d559273
2 changed files with 128 additions and 2 deletions

View file

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

View file

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