[FIX] smoke: name the URL that actually failed, keep the previous log
The report named the sitemap URL, never the one the redirect chain ended on. On this site every page goes through two or three hops -- measured, 146 for 55 pages, between the language prefix and the canonical slug -- so a 500 at the end was reported against a page that answers perfectly well. One goes and checks it, finds it healthy, and concludes the tool is wrong. urllib carries the answer: HTTPError.url is the URL that produced the error, not the one requested. fetch now returns it and the report shows it when it differs. A timeout was never reported as 500 -- fetch returns 0 and the report writes "no answer" -- but the evidence for a real 500 did evaporate: the Odoo log was opened with "w", so replaying the test erased the trace of the failure one had just seen. One generation is kept now. --- FR --- Le rapport nommait l'URL du sitemap, jamais celle où la chaîne de redirections aboutit. Sur ce site chaque page en traverse deux ou trois — mesuré, 146 pour 55 pages — donc un 500 au bout était imputé à une page qui répond très bien. On va la vérifier, on la trouve saine, et l'on conclut que l'outil se trompe. urllib porte la réponse : HTTPError.url est l'URL qui a produit l'erreur. `fetch` la rend, et le rapport l'affiche quand elle diffère. Un dépassement de délai n'a jamais été rendu comme un 500 — `fetch` rend 0, écrit « aucune réponse » — mais la preuve d'un vrai 500 s'évaporait : le journal Odoo était ouvert en « w », donc rejouer le test effaçait la trace qu'on venait de voir. Une génération est gardée. Assisted-by: Claude Opus 5
This commit is contained in:
parent
e7c54c0a15
commit
437ac4b457
4 changed files with 257 additions and 28 deletions
|
|
@ -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 "<sitemapindex" in body.lower():
|
||||
lst_page = []
|
||||
for loc in lst_loc:
|
||||
_status, sub = fetch(local_url(base_url, loc))
|
||||
_status, sub, _fin = fetch(local_url(base_url, loc))
|
||||
lst_page.extend(RE_LOC.findall(sub))
|
||||
lst_loc = lst_page
|
||||
seen, lst_url = set(), []
|
||||
|
|
@ -252,7 +274,7 @@ def attach_missing_parents(lst_failure, lst_log):
|
|||
ce qui n'avait été rattaché à rien — mieux vaut un coupable mal attribué
|
||||
qu'un coupable perdu.
|
||||
"""
|
||||
known = {pid for _u, _s, lst in lst_failure for pid in lst}
|
||||
known = {pid for _u, _s, lst, _f in lst_failure for pid in lst}
|
||||
extra = []
|
||||
for line in lst_log:
|
||||
# LES DEUX : le parent ET l'enfant. Mesuré sur /contactus — le
|
||||
|
|
@ -267,19 +289,22 @@ def attach_missing_parents(lst_failure, lst_log):
|
|||
return lst_failure
|
||||
rebuilt = []
|
||||
placed = False
|
||||
for url, status, lst_parent in lst_failure:
|
||||
for url, status, lst_parent, finale in lst_failure:
|
||||
if not lst_parent and not placed:
|
||||
lst_parent = list(extra)
|
||||
placed = True
|
||||
rebuilt.append((url, status, lst_parent))
|
||||
rebuilt.append((url, status, lst_parent, finale))
|
||||
if not placed and rebuilt:
|
||||
url, status, lst_parent = rebuilt[0]
|
||||
rebuilt[0] = (url, status, lst_parent + extra)
|
||||
url, status, lst_parent, finale = rebuilt[0]
|
||||
rebuilt[0] = (url, status, lst_parent + extra, finale)
|
||||
return rebuilt
|
||||
|
||||
|
||||
def check_urls(lst_url, timeout=30):
|
||||
"""[(url, statut, [])] pour celles qui ont échoué.
|
||||
"""[(url, statut, [vues], url finale)] pour celles qui ont échoué.
|
||||
|
||||
TOUJOURS quatre éléments, le dernier étant l'URL réellement atteinte.
|
||||
Un tuple de taille variable obligerait chaque lecteur à s'en méfier.
|
||||
|
||||
Les vues en cause sont rattachées après coup, en relisant le journal du
|
||||
serveur : elles y arrivent quand Odoo vide son tampon, pas quand la
|
||||
|
|
@ -287,9 +312,9 @@ def check_urls(lst_url, timeout=30):
|
|||
"""
|
||||
lst_failure = []
|
||||
for url in lst_url:
|
||||
status, _body = fetch(url, timeout=timeout)
|
||||
status, _body, finale = fetch(url, timeout=timeout)
|
||||
if status == 0 or status >= 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')} :"
|
||||
|
|
|
|||
|
|
@ -6191,6 +6191,10 @@ TRANSLATIONS = {
|
|||
"fr": "vue(s) portent encore une balise <tree> qu'Odoo 18",
|
||||
"en": "view(s) still carry a <tree> tag Odoo 18",
|
||||
},
|
||||
"failed at": {
|
||||
"fr": "a échoué sur",
|
||||
"en": "failed at",
|
||||
},
|
||||
"Census": {
|
||||
"fr": "Recensement",
|
||||
"en": "Census",
|
||||
|
|
|
|||
175
test/test_smoke_final_url.py
Normal file
175
test/test_smoke_final_url.py
Normal file
|
|
@ -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()
|
||||
|
|
@ -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,
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in a new issue