diff --git a/script/odoo/migration/smoke_public_url.py b/script/odoo/migration/smoke_public_url.py index 501fe76..37eccf8 100755 --- a/script/odoo/migration/smoke_public_url.py +++ b/script/odoo/migration/smoke_public_url.py @@ -138,6 +138,15 @@ def start_server(database, port, config_path="./config.conf", log_path=None): nomme la vue fautive faisait partie des absentes. Un fichier se relit entièrement, quand on veut. """ + # Garder l'exécution PRÉCÉDENTE. Le journal était ouvert en « w » : + # relancer le test effaçait la trace de l'échec qu'on venait de voir, + # et il ne restait plus rien à examiner. Une seule génération suffit — + # c'est celle d'avant qu'on vient chercher. + if log_path and os.path.isfile(log_path): + try: + os.replace(log_path, log_path + ".1") + except OSError: + pass handle = open(log_path, "w", encoding="utf-8") if log_path else None server = subprocess.Popen( [ @@ -179,23 +188,36 @@ def read_log(log_path): def fetch(url, timeout=30): - """(statut, corps). Statut 0 quand la connexion elle-même échoue.""" + """(statut, corps, url finale). Statut 0 si la connexion échoue. + + L'URL FINALE, pas seulement celle qu'on a demandée. Sur ce site + chaque page traverse deux ou trois redirections — mesuré, 146 pour + 55 pages — et quand la dernière rend 500, l'outil nommait la + première. On allait vérifier une page saine et l'on concluait que le + test se trompait. + """ try: with urllib.request.urlopen(url, timeout=timeout) as answer: - return answer.getcode(), answer.read().decode( - "utf-8", errors="replace" + return ( + answer.getcode(), + answer.read().decode("utf-8", errors="replace"), + answer.geturl(), ) except urllib.error.HTTPError as exc: - return exc.code, exc.read().decode("utf-8", errors="replace") + return ( + exc.code, + exc.read().decode("utf-8", errors="replace"), + exc.url or url, + ) except Exception: - return 0, "" + return 0, "", url def wait_ready(base_url, timeout=180, sleep=2): """Attendre que le serveur réponde. False s'il n'est jamais venu.""" deadline = time.time() + timeout while time.time() < deadline: - status, _body = fetch(base_url + "/web/login", timeout=5) + status, _body, _fin = fetch(base_url + "/web/login", timeout=5) if status: return True time.sleep(sleep) @@ -209,7 +231,7 @@ def sitemap_urls(base_url): servie en local. Garder le domaine ferait interroger la production — c'est le genre d'erreur qui ne se voit qu'après. """ - status, body = fetch(base_url + "/sitemap.xml") + status, body, _fin = fetch(base_url + "/sitemap.xml") if not status or status >= 400: return [], status lst_loc = RE_LOC.findall(body) @@ -217,7 +239,7 @@ def sitemap_urls(base_url): if "= 400: - lst_failure.append((url, status, [])) + lst_failure.append((url, status, [], finale)) return lst_failure @@ -324,7 +349,7 @@ def culprit_keys(database, lst_failure): exactement la recopie où l'on se trompe. """ lst_id = [] - for _url, _status, lst_parent in lst_failure: + for _url, _status, lst_parent, _finale in lst_failure: for parent_id in lst_parent: if parent_id not in lst_id: lst_id.append(parent_id) @@ -355,9 +380,14 @@ def render(lst_url, lst_failure, lst_key=None): f"❌ {len(lst_failure)} {t('of')} {len(lst_url)}" f" {t('public URL(s) failed')} :" ] - for url, status, lst_parent in lst_failure: + for url, status, lst_parent, finale in lst_failure: label = status or t("no answer") lines.append(f" [{label}] {url}") + # L'URL du sitemap n'est pas celle qui a échoué quand une + # redirection s'est interposée. Ne montrer que la première + # envoyait vérifier une page saine. + if finale and finale != url: + lines.append(f" → {t('failed at')} {finale}") if lst_parent: lines.append( f" {t('parent view(s) in cause')} :" diff --git a/script/todo/todo_i18n.py b/script/todo/todo_i18n.py index 6a5118a..6983f5c 100644 --- a/script/todo/todo_i18n.py +++ b/script/todo/todo_i18n.py @@ -6191,6 +6191,10 @@ TRANSLATIONS = { "fr": "vue(s) portent encore une balise qu'Odoo 18", "en": "view(s) still carry a tag Odoo 18", }, + "failed at": { + "fr": "a échoué sur", + "en": "failed at", + }, "Census": { "fr": "Recensement", "en": "Census", diff --git a/test/test_smoke_final_url.py b/test/test_smoke_final_url.py new file mode 100644 index 0000000..85a82dc --- /dev/null +++ b/test/test_smoke_final_url.py @@ -0,0 +1,175 @@ +#!/usr/bin/env python3 +# © 2021-2026 TechnoLibre (http://www.technolibre.ca) +# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl) + +"""L'URL qui a échoué n'est pas toujours celle qu'on a demandée. + +Sur un site Odoo, chaque page traverse deux ou trois redirections — +mesuré, 146 pour 55 pages, entre la langue et le slug canonique. Quand +la DERNIÈRE rend 500, l'outil nommait la première : on allait vérifier +une page parfaitement saine et l'on concluait que le test se trompait. + +Et un dépassement de délai ne rend PAS 500 : `fetch` rend 0, que le +rapport écrit « aucune réponse ». Confondre les deux enverrait chercher +une lenteur là où le serveur a répondu par une erreur. +""" + +import http.server +import io +import os +import socketserver +import sys +import threading +import unittest + +sys.path.append( + os.path.normpath(os.path.join(os.path.dirname(__file__), "..")) +) + +from script.odoo.migration import smoke_public_url as smoke # noqa: E402 +from script.todo import todo_i18n # noqa: E402 + + +class Chaine(http.server.BaseHTTPRequestHandler): + """/depart → 303 → /milieu → 303 → /fin, qui décide.""" + + fin_status = 500 + + def do_GET(self): + if self.path == "/depart": + self.send_response(303) + self.send_header("Location", "/milieu") + self.end_headers() + elif self.path == "/milieu": + self.send_response(303) + self.send_header("Location", "/fin") + self.end_headers() + elif self.path == "/direct": + self.send_response(500) + self.end_headers() + self.wfile.write(b"boum") + else: + self.send_response(self.fin_status) + self.end_headers() + self.wfile.write(b"boum") + + def log_message(self, *args): + pass + + +class TestFetchFollowsTheChain(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls.srv = socketserver.TCPServer(("127.0.0.1", 0), Chaine) + cls.port = cls.srv.server_address[1] + cls.fil = threading.Thread(target=cls.srv.serve_forever, daemon=True) + cls.fil.start() + + @classmethod + def tearDownClass(cls): + cls.srv.shutdown() + cls.srv.server_close() + + def url(self, chemin): + return f"http://127.0.0.1:{self.port}{chemin}" + + def test_it_reports_the_url_that_actually_failed(self): + # LE point : la 500 est au bout de la chaîne, pas au départ. + statut, _corps, finale = smoke.fetch(self.url("/depart"), timeout=5) + self.assertEqual(statut, 500) + self.assertTrue(finale.endswith("/fin"), finale) + + def test_without_a_redirect_both_are_the_same(self): + statut, _corps, finale = smoke.fetch(self.url("/direct"), timeout=5) + self.assertEqual(statut, 500) + self.assertEqual(finale, self.url("/direct")) + + def test_a_success_also_carries_its_final_url(self): + Chaine.fin_status = 200 + try: + statut, _corps, finale = smoke.fetch( + self.url("/depart"), timeout=5 + ) + self.assertEqual(statut, 200) + self.assertTrue(finale.endswith("/fin"), finale) + finally: + Chaine.fin_status = 500 + + def test_a_dead_host_is_zero_NOT_five_hundred(self): + # C'est ce qui distingue « le serveur a répondu par une erreur » + # de « il n'a pas répondu ». Les confondre envoie chercher une + # lenteur là où il y a une exception. + statut, corps, finale = smoke.fetch( + "http://127.0.0.1:1/jamais", timeout=1 + ) + self.assertEqual(statut, 0) + self.assertEqual(corps, "") + self.assertEqual(finale, "http://127.0.0.1:1/jamais") + + def test_check_urls_keeps_the_final_url(self): + echecs = smoke.check_urls([self.url("/depart")], timeout=5) + self.assertEqual(len(echecs), 1) + url, statut, parents, finale = echecs[0] + self.assertEqual(url, self.url("/depart")) + self.assertEqual(statut, 500) + self.assertEqual(parents, []) + self.assertTrue(finale.endswith("/fin")) + + def test_a_page_that_answers_is_not_a_failure(self): + Chaine.fin_status = 200 + try: + self.assertEqual( + smoke.check_urls([self.url("/depart")], timeout=5), [] + ) + finally: + Chaine.fin_status = 500 + + +class TestTheReport(unittest.TestCase): + def test_it_names_the_final_url_when_it_differs(self): + texte = smoke.render( + ["a", "b"], [("http://x/depart", 500, [], "http://x/fin")] + ) + self.assertIn("http://x/depart", texte) + self.assertIn(todo_i18n.t("failed at"), texte) + self.assertIn("http://x/fin", texte) + + def test_it_stays_quiet_when_they_are_the_same(self): + # Répéter la même URL sur deux lignes n'apprend rien et allonge + # un rapport qui peut compter trente-quatre entrées. + texte = smoke.render( + ["a"], [("http://x/page", 500, [], "http://x/page")] + ) + self.assertNotIn(todo_i18n.t("failed at"), texte) + + def test_no_answer_is_worded_apart_from_a_status(self): + texte = smoke.render( + ["a"], [("http://x/page", 0, [], "http://x/page")] + ) + self.assertIn(todo_i18n.t("no answer"), texte) + self.assertNotIn("[500]", texte) + + +class TestTheLogSurvives(unittest.TestCase): + def test_the_previous_run_is_kept(self): + # Le journal était ouvert en « w » : relancer le test effaçait la + # trace de l'échec qu'on venait de voir. + with io.open( + os.path.join( + os.path.dirname(__file__), + "..", + "script", + "odoo", + "migration", + "smoke_public_url.py", + ), + encoding="utf-8", + ) as handle: + src = handle.read() + debut = src.index("def start_server") + fin = src.index("subprocess.Popen", debut) + self.assertIn("os.replace(log_path, log_path", src[debut:fin]) + + +if __name__ == "__main__": + unittest.main() diff --git a/test/test_smoke_public_url.py b/test/test_smoke_public_url.py index 8d2c9db..3c0977c 100755 --- a/test/test_smoke_public_url.py +++ b/test/test_smoke_public_url.py @@ -53,7 +53,15 @@ class TestReadingTheSitemap(unittest.TestCase): def setUp(self): self.answers = {} self.original = smoke.fetch - smoke.fetch = lambda url, timeout=30: self.answers.get(url, (404, "")) + + # `fetch` rend TROIS valeurs depuis qu'il porte l'URL finale. + # Un faux resté à deux casse chaque appelant sur un « not enough + # values to unpack » qui n'apprend rien de la panne réelle. + def faux(url, timeout=30): + statut, corps = self.answers.get(url, (404, "")) + return statut, corps, url + + smoke.fetch = faux self.addCleanup(setattr, smoke, "fetch", self.original) def test_a_plain_sitemap(self): @@ -101,13 +109,22 @@ class TestWhatCountsAsAFailure(unittest.TestCase): def setUp(self): self.answers = {} self.original = smoke.fetch - smoke.fetch = lambda url, timeout=30: self.answers.get(url, (200, "")) + + # `fetch` rend TROIS valeurs depuis qu'il porte l'URL finale. + # Un faux resté à deux casse chaque appelant sur un « not enough + # values to unpack » qui n'apprend rien de la panne réelle. + def faux(url, timeout=30): + statut, corps = self.answers.get(url, (200, "")) + return statut, corps, url + + smoke.fetch = faux self.addCleanup(setattr, smoke, "fetch", self.original) def test_a_500_fails(self): self.answers["http://h/bad"] = (500, "") self.assertEqual( - smoke.check_urls(["http://h/bad"]), [("http://h/bad", 500, [])] + smoke.check_urls(["http://h/bad"]), + [("http://h/bad", 500, [], "http://h/bad")], ) def test_a_404_fails_too(self): @@ -119,7 +136,8 @@ class TestWhatCountsAsAFailure(unittest.TestCase): def test_no_answer_at_all_fails(self): self.answers["http://h/dead"] = (0, "") self.assertEqual( - smoke.check_urls(["http://h/dead"]), [("http://h/dead", 0, [])] + smoke.check_urls(["http://h/dead"]), + [("http://h/dead", 0, [], "http://h/dead")], ) def test_a_200_passes(self): @@ -141,7 +159,9 @@ class TestTheReport(unittest.TestCase): self.assertIn("✅", text) def test_a_failure_shows_the_status_and_the_url(self): - text = smoke.render(["a"], [("http://h/blog/x", 500, [])]) + text = smoke.render( + ["a"], [("http://h/blog/x", 500, [], "http://h/blog/x")] + ) self.assertIn("500", text) self.assertIn("http://h/blog/x", text) @@ -211,7 +231,7 @@ class TestTheCulpritViewsAreNamed(unittest.TestCase): def test_a_late_context_is_still_attached(self): # Odoo vide son tampon à l'arrêt : le journal se lit APRÈS, et rien # ne doit dépendre du moment où la ligne est apparue. - lst_failure = [("http://h/a", 500, [])] + lst_failure = [("http://h/a", 500, [], "http://h/a")] log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) self.assertIn("2841", rebuilt[0][2]) @@ -221,20 +241,20 @@ class TestTheCulpritViewsAreNamed(unittest.TestCase): # à sa vue module, et c'est l'enfant (3282) qui portait l'arch # périmée. Ne nommer que le parent envoyait réinitialiser une copie # qui allait déjà bien, et la page restait en 500. - lst_failure = [("http://h/contactus", 500, [])] + lst_failure = [("http://h/contactus", 500, [], "http://h/contactus")] log = ["[view_id: 3282, model: n/a, parent_id: 3281]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) self.assertEqual(rebuilt[0][2], ["3281", "3282"]) def test_the_parent_comes_first(self): # C'est le cas le plus fréquent — le blogue — donc en tête de liste. - lst_failure = [("http://h/a", 500, [])] + lst_failure = [("http://h/a", 500, [], "http://h/a")] log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) self.assertEqual(rebuilt[0][2][0], "2841") def test_an_already_attributed_id_is_not_duplicated(self): - lst_failure = [("http://h/a", 500, ["2841"])] + lst_failure = [("http://h/a", 500, ["2841"], "http://h/a")] log = ["[view_id: 3288, model: n/a, parent_id: 2841]"] rebuilt = smoke.attach_missing_parents(lst_failure, log) self.assertEqual(rebuilt[0][2].count("2841"), 1) @@ -359,7 +379,7 @@ class TestOfferingTheFix(unittest.TestCase): with contextlib.redirect_stdout(out): done = smoke.prompt( "db", - [("http://h/a", 500, ["2841"])], + [("http://h/a", 500, ["2841"], "http://h/a")], lst_key, ask=lambda prompt: answer, )