diff --git a/forgetest/forgetest/catalog.py b/forgetest/forgetest/catalog.py index 06d91f1..e9fb64f 100644 --- a/forgetest/forgetest/catalog.py +++ b/forgetest/forgetest/catalog.py @@ -39,6 +39,7 @@ class Test: self.description = description or (fn.__doc__ or "").strip() self.fn = fn self._source_sha = None + self._fp = (None, None) # (manifest content sha, fingerprint) @property def source_sha(self): @@ -51,7 +52,17 @@ class Test: return self._source_sha def fingerprint(self, manifest): - return _manifest.fingerprint(manifest, self.covers, extra=[self.source_sha]) + """The domain fingerprint on this manifest, memoized by the + manifest's content hash: the page recomputes every test's + fingerprint on every poll, and a manifest never changes under a + running tool.""" + key = manifest.content_sha + if key and self._fp[0] == key: + return self._fp[1] + fp = _manifest.fingerprint(manifest, self.covers, extra=[self.source_sha]) + if key: + self._fp = (key, fp) + return fp def definition(self): """The gate-visible definition (no implementation, no prose).""" diff --git a/forgetest/forgetest/log.py b/forgetest/forgetest/log.py index ce2a47c..27e8c27 100644 --- a/forgetest/forgetest/log.py +++ b/forgetest/forgetest/log.py @@ -13,6 +13,11 @@ One JSON object per line under the data directory (default Every record carries "t" (type) and "ts" (UTC, ISO 8601, seconds). The file is only ever appended; a corrupt line is skipped, counted, and reported, never repaired in place. + +Because the file only grows, `read` parses each line once and keeps what +it parsed; a later call reads only the bytes appended since. A result +record carries the whole run log, so the file reaches megabytes over a +campaign, and the page asks for the state every second or two. """ import json import os @@ -35,6 +40,9 @@ class Log: self.path = path or os.path.join(data_dir(), "results.jsonl") self._lock = threading.Lock() self.corrupt = 0 + self._recs = [] # every record parsed so far, in file order + self._offset = 0 # bytes of the file already consumed + self._tail = b"" # bytes past the last newline: not a record yet def append(self, rec): rec = dict(rec) @@ -48,28 +56,54 @@ class Log: os.fsync(f.fileno()) return rec - def read(self): - recs = [] + def _forget(self): + """Drop what was parsed: the next read starts from the top.""" + self._recs = [] + self._offset = 0 + self._tail = b"" self.corrupt = 0 - if not os.path.exists(self.path): - return recs - with self._lock: - with open(self.path, "r", encoding="utf-8") as f: - lines = f.readlines() - for line in lines: + + def _consume(self, chunk): + """Parse the whole lines in `chunk`, holding back a partial tail.""" + data = self._tail + chunk + cut = data.rfind(b"\n") + if cut < 0: + self._tail = data + return + self._tail = data[cut + 1:] + for line in data[:cut].split(b"\n"): line = line.strip() if not line: continue try: - rec = json.loads(line) - except ValueError: + rec = json.loads(line.decode("utf-8")) + except (ValueError, UnicodeDecodeError): self.corrupt += 1 continue if isinstance(rec, dict) and "t" in rec: - recs.append(rec) + self._recs.append(rec) else: self.corrupt += 1 - return recs + + def read(self): + """Every record, in file order. Only the bytes appended since the + last call are parsed; a file that shrank or was replaced is read + again from the top.""" + with self._lock: + try: + size = os.path.getsize(self.path) + except OSError: + self._forget() + return [] + if size < self._offset: + self._forget() + if size != self._offset: + with open(self.path, "rb") as f: + f.seek(self._offset) + chunk = f.read() + self._offset += len(chunk) + self._consume(chunk) + return list(self._recs) def raw(self): if not os.path.exists(self.path): diff --git a/forgetest/forgetest/manifest.py b/forgetest/forgetest/manifest.py index 561909a..3930a43 100644 --- a/forgetest/forgetest/manifest.py +++ b/forgetest/forgetest/manifest.py @@ -10,6 +10,7 @@ plus the test's own implementation. A recorded PASS applies to a build exactly when the fingerprint recomputed from that build's manifest is the same - the same code runs on the board and in the release gate. """ +import functools import hashlib import json import os @@ -32,9 +33,11 @@ def sha256_text(text): return hashlib.sha256(text.encode("utf-8")).hexdigest() +@functools.lru_cache(maxsize=512) def glob_to_regex(pattern): """Coverage glob -> anchored regex. '**' spans directories, '*' and '?' - stay inside one path segment. Paths use '/' (git paths).""" + stay inside one path segment. Paths use '/' (git paths). Cached: the + catalog matches the same few hundred globs over and over.""" out = [] i, n = 0, len(pattern) while i < n: @@ -71,6 +74,7 @@ class Manifest: self.platform = data.get("platform", {}) or {} self.image = data.get("image", {}) or {} self.content_sha = data.get("content_sha256") + self._files = {} @classmethod def load(cls, path=None): @@ -91,12 +95,15 @@ class Manifest: return self.image.get("name") or "unknown" def files(self, component): - """The [path, blob] list of a component, or None if the component - is not in this manifest.""" + """The (path, blob) pairs of a component, or None if the component + is not in this manifest. Built once per component: the manifest is + immutable and every coverage glob asks for the same lists.""" + if component in self._files: + return self._files[component] c = self.components.get(component) - if c is None: - return None - return [tuple(x) for x in c.get("files", [])] + out = None if c is None else [tuple(x) for x in c.get("files", [])] + self._files[component] = out + return out def component_names(self): return sorted(self.components) diff --git a/forgetest/forgetest/page.py b/forgetest/forgetest/page.py index 2850261..2f839e6 100644 --- a/forgetest/forgetest/page.py +++ b/forgetest/forgetest/page.py @@ -1,8 +1,16 @@ """The single page: acceptance tab + bench tab. Self-contained (inline CSS/JS, ES5, no external assets); the visual identity follows the -forgectrl control panel. State comes from GET /state on a 2 s poll; the -catalog and the bench listing are fetched once per tab and refreshed -after a run finishes.""" +forgectrl control panel. State comes from GET /state, polled every 2 s +when the machine is idle and every second during a run; the catalog and +the bench listing are fetched once per tab and refreshed after a run +finishes. + +Rows, prompt buttons and tool entries are built once and thereafter only +updated in place. A poll that rebuilt them would swallow the click it +landed on: the button that took the mousedown would be gone before the +mouseup, so no click event would ever be raised. Every action also greys +its control out on the press and pulls the next poll forward, so the page +answers the operator rather than the timer.""" _HTML = r""" @@ -81,7 +89,7 @@ pre#log{background:#1d1e26;color:#d7dae0;font-family:ui-monospace,Consolas,monos
- + @@ -120,107 +128,208 @@ pre#log{background:#1d1e26;color:#d7dae0;font-family:ui-monospace,Consolas,monos diff --git a/forgetest/forgetest/server.py b/forgetest/forgetest/server.py index 07228a4..697dce1 100644 --- a/forgetest/forgetest/server.py +++ b/forgetest/forgetest/server.py @@ -9,7 +9,10 @@ page. Read-only calls need the origin checks only. Routes GET / the page - GET /state campaign + per-test state + running run + GET /state campaign + per-test state + running run. + Carries an ETag; If-None-Match on an + unchanged state gets a 304, which is what an + idle page polls for. GET /catalog test definitions (title, steps, covers...) GET /bench bench tool listing GET /result?test&ts one full result record (log, evidence) @@ -23,6 +26,7 @@ Routes POST /reset {reason} POST /export build + save the artifact, returns it """ +import hashlib import hmac import json import os @@ -96,6 +100,11 @@ class App: class Handler(BaseHTTPRequestHandler): server_version = "forgetest" + # Keep-alive: the page polls, so a handshake and a fresh thread per + # request is pure overhead. Idle connections are dropped after the + # timeout rather than holding a thread forever. + protocol_version = "HTTP/1.1" + timeout = 30 app = None # set on the server class # -- plumbing --------------------------------------------------------- @@ -108,6 +117,7 @@ class Handler(BaseHTTPRequestHandler): body = json.dumps(body, sort_keys=True).encode("utf-8") elif isinstance(body, str): body = body.encode("utf-8") + self.responded = True self.send_response(status) self.send_header("Content-Type", ctype + ("; charset=utf-8" if ctype.startswith("text/") else "")) self.send_header("Content-Length", str(len(body))) @@ -119,8 +129,25 @@ class Handler(BaseHTTPRequestHandler): if self.command != "HEAD": self.wfile.write(body) + def _send_304(self, etag): + self.responded = True + self.send_response(304) + self.send_header("ETag", etag) + self.send_header("Cache-Control", "no-store") + self.send_header("Content-Length", "0") + self.end_headers() + def _deny(self, status, msg): - self._send(status, {"error": msg}) + # One response per request: on a kept-alive connection a second + # one would desynchronize the stream. + if getattr(self, "responded", False): + return + # A refused POST still has its body in the socket. Leaving it + # there would make the next read on this connection take the + # body for a request line, so the connection ends here. + # send_header sets close_connection from this. + extra = None if getattr(self, "body_read", True) else {"Connection": "close"} + self._send(status, {"error": msg}, extra=extra) def _read_ok(self): if not origin_ok(self.headers): @@ -141,10 +168,12 @@ class Handler(BaseHTTPRequestHandler): def _body_json(self): n = int(self.headers.get("Content-Length") or 0) if n <= 0: + self.body_read = True return {} if n > 1 << 20: raise ValueError("body too large") raw = self.rfile.read(n) + self.body_read = True ctype = self.headers.get("Content-Type", "") if "json" in ctype: data = json.loads(raw.decode("utf-8")) @@ -161,14 +190,22 @@ class Handler(BaseHTTPRequestHandler): path = url.path query = urllib.parse.parse_qs(url.query) r = self.app.runner + self.responded = False if not self._read_ok(): return try: if path == "/": self._send(200, _page.render(self.app.token), "text/html") elif path == "/state": + # Conditional: an idle page polls an unchanged state, and + # a 304 spares both ends the payload and the re-render. state, _ = r.state() - self._send(200, state) + body = json.dumps(state, sort_keys=True).encode("utf-8") + etag = '"%s"' % hashlib.sha256(body).hexdigest()[:32] + if self.headers.get("If-None-Match") == etag: + self._send_304(etag) + else: + self._send(200, body, extra={"ETag": etag}) elif path == "/catalog": self._send(200, {"tests": [t.describe() for t in r.tests()], "catalog_hash": r.catalog_hash}) @@ -210,6 +247,8 @@ class Handler(BaseHTTPRequestHandler): path = url.path query = urllib.parse.parse_qs(url.query) r = self.app.runner + self.responded = False + self.body_read = False if not self._write_ok(query): return try: diff --git a/forgetest/tests/test_responsiveness.py b/forgetest/tests/test_responsiveness.py new file mode 100644 index 0000000..1ec39ad --- /dev/null +++ b/forgetest/tests/test_responsiveness.py @@ -0,0 +1,199 @@ +"""What keeps the page answering the operator instead of the timer. + +Two things went wrong on the bench and are pinned here: + + - the state cost. Every poll re-read and re-parsed the whole result log, + and recomputed every test's domain fingerprint. A result record carries + its run log, so the file reaches megabytes over a campaign and the poll + grew with it. Both are now parsed and computed once. + - the wasted payload. An idle page polls an unchanged state; it now gets + a 304 instead of the whole thing. + +The click-swallowing defect these were found with lives in the page's +JavaScript and is not reachable from here: rows, prompt buttons and tool +entries are built once and afterwards only updated in place, because a +poll that rebuilt them removed the button between the operator's mousedown +and mouseup, and no click event was ever raised. `test_page_never_rebuilds` +holds the shape of that rule. +""" +import json +import os +import shutil +import tempfile +import unittest + +import helpers +from forgetest import catalog, manifest as manifest_mod, page +from forgetest.log import Log + + +def t_noop(ctx): + pass + + +class LogCacheTests(unittest.TestCase): + """The log is append-only, so a read parses each line exactly once.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="forgetest-log-") + self.path = os.path.join(self.tmp, "results.jsonl") + self.log = Log(self.path) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def write(self, *lines): + with open(self.path, "a", encoding="utf-8") as f: + for line in lines: + f.write(line + "\n") + + def test_missing_file_reads_empty(self): + self.assertEqual(self.log.read(), []) + + def test_appends_are_picked_up(self): + self.log.append({"t": "campaign", "id": "c1"}) + self.assertEqual([r["id"] for r in self.log.read()], ["c1"]) + self.log.append({"t": "result", "test": "a.b"}) + self.log.append({"t": "result", "test": "c.d"}) + recs = self.log.read() + self.assertEqual([r["t"] for r in recs], ["campaign", "result", "result"]) + self.assertEqual(recs[2]["test"], "c.d") + + def test_only_new_bytes_are_parsed(self): + for i in range(5): + self.log.append({"t": "result", "test": "t%d" % i}) + self.log.read() + # a second read must not touch the file at all + real_open = open + opened = [] + + def counting_open(*a, **kw): + opened.append(a[0]) + return real_open(*a, **kw) + + import builtins + builtins.open = counting_open + try: + recs = self.log.read() + finally: + builtins.open = real_open + self.assertEqual(opened, [], "an unchanged log was re-opened") + self.assertEqual(len(recs), 5) + + def test_read_returns_a_fresh_list(self): + """Callers filter and sort the result; the cache must not be theirs.""" + self.log.append({"t": "result", "test": "a.b"}) + first = self.log.read() + first.append({"t": "bogus"}) + self.assertEqual(len(self.log.read()), 1) + + def test_partial_trailing_line_is_held_not_counted_corrupt(self): + self.write('{"t":"result","test":"a.b"}') + with open(self.path, "a", encoding="utf-8") as f: + f.write('{"t":"result","te') # a line still being written + recs = self.log.read() + self.assertEqual(len(recs), 1) + self.assertEqual(self.log.corrupt, 0) + with open(self.path, "a", encoding="utf-8") as f: + f.write('st":"c.d"}\n') # ... now finished + recs = self.log.read() + self.assertEqual([r["test"] for r in recs], ["a.b", "c.d"]) + self.assertEqual(self.log.corrupt, 0) + + def test_corrupt_lines_counted_once_not_per_read(self): + self.write('{"t":"result","test":"a.b"}', "not json at all", '["not","a","record"]', + '{"t":"result","test":"c.d"}') + recs = self.log.read() + self.assertEqual([r["test"] for r in recs], ["a.b", "c.d"]) + self.assertEqual(self.log.corrupt, 2) + self.log.read() + self.log.read() + self.assertEqual(self.log.corrupt, 2, "corrupt lines re-counted on every read") + + def test_replaced_file_is_read_again(self): + self.write('{"t":"result","test":"a.b"}', '{"t":"result","test":"c.d"}') + self.assertEqual(len(self.log.read()), 2) + with open(self.path, "w", encoding="utf-8") as f: + f.write('{"t":"campaign","id":"c9"}\n') + recs = self.log.read() + self.assertEqual([r.get("id") for r in recs], ["c9"]) + + def test_blank_lines_are_not_corrupt(self): + self.write('{"t":"result","test":"a.b"}', "", " ", '{"t":"result","test":"c.d"}') + self.assertEqual(len(self.log.read()), 2) + self.assertEqual(self.log.corrupt, 0) + + +class FingerprintCacheTests(unittest.TestCase): + """Memoized per manifest: the page recomputes every test's fingerprint + on every poll, and a manifest never changes under a running tool.""" + + def setUp(self): + self.man = helpers.make_manifest() + self.t = helpers.make_test("fake.fp", [("forgectrl", "src/ui.c")], fn=t_noop) + + def test_repeat_calls_agree(self): + first = self.t.fingerprint(self.man) + self.assertEqual(self.t.fingerprint(self.man), first) + self.assertEqual(self.t.fingerprint(self.man), first) + + def test_a_changed_manifest_is_not_served_from_the_cache(self): + first = self.t.fingerprint(self.man) + man2 = helpers.with_file(self.man, "forgectrl", "src/ui.c", "ui v2") + self.assertNotEqual(self.t.fingerprint(man2), first) + # and back again: the cache must not have latched the new one either + self.assertEqual(self.t.fingerprint(self.man), first) + + def test_a_file_outside_the_coverage_does_not_move_it(self): + first = self.t.fingerprint(self.man) + man2 = helpers.with_file(self.man, "forgectrl", "src/cool.c", "cool v2") + self.assertEqual(self.t.fingerprint(man2), first) + + def test_manifest_without_a_content_hash_is_not_cached(self): + """The cache is keyed by the manifest's content hash. A manifest + that has none must still fingerprint correctly, not collide.""" + a = manifest_mod.Manifest({"components": {"forgectrl": {"files": [["src/ui.c", "aaa"]]}}, + "platform": {}}) + b = manifest_mod.Manifest({"components": {"forgectrl": {"files": [["src/ui.c", "bbb"]]}}, + "platform": {}}) + self.assertIsNone(a.content_sha) + self.assertNotEqual(self.t.fingerprint(a), self.t.fingerprint(b)) + + def test_component_file_list_is_stable_and_read_only(self): + files = self.man.files("forgectrl") + self.assertEqual(files, self.man.files("forgectrl")) + self.assertIsNone(self.man.files("no-such-component")) + + +class PageTests(unittest.TestCase): + def test_page_never_rebuilds_what_the_operator_may_be_pressing(self): + """A poll must update rows, prompt buttons and tool entries in + place. Assigning innerHTML to their containers on every poll is + what swallowed the clicks; only the one-time build may do it.""" + html = page.render("0" * 32) + for container, builder in (("groups", "buildGroups"), ("tools", "buildBench")): + self.assertIn("function %s(" % builder, html, + "no one-time builder for #%s" % container) + assigns = html.count("$('%s').innerHTML=" % container) + self.assertEqual(assigns, 1, + "#%s is assigned innerHTML %d times; it belongs to the " + "builder alone" % (container, assigns)) + # the per-poll path writes through the guarded setters only + self.assertIn("function setHtml(e,h){if(e&&e.__h!==h)", html) + self.assertIn("function updateGroups()", html) + self.assertIn("function updateBench()", html) + # the prompt buttons are rebuilt only when the prompt changes + self.assertIn("if(pk!==promptKey)", html) + + def test_page_is_self_contained_ascii(self): + html = page.render("0" * 32) + self.assertNotIn("__TOKEN__", html) + html.encode("ascii") # no stray typography in an embedded page + stray = sorted(set(hex(ord(c)) for c in html if ord(c) < 32 and c != "\n")) + self.assertEqual(stray, [], "control characters in the page source") + for remote in ("http://", "https://", "//cdn"): + self.assertNotIn(remote, html) + + +if __name__ == "__main__": + unittest.main() diff --git a/forgetest/tests/test_server.py b/forgetest/tests/test_server.py index cd18038..b540c89 100644 --- a/forgetest/tests/test_server.py +++ b/forgetest/tests/test_server.py @@ -97,6 +97,11 @@ class ServerTests(unittest.TestCase): # -- helpers ---------------------------------------------------------- def call(self, method, path, body=None, token=True, headers=None): + st, payload, _ = self.call_h(method, path, body, token, headers) + return st, payload + + def call_h(self, method, path, body=None, token=True, headers=None): + """As call, plus the response headers.""" url = "http://127.0.0.1:%d%s" % (self.port, path) hdrs = {"Host": "127.0.0.1:%d" % self.port} if token: @@ -112,14 +117,15 @@ class ServerTests(unittest.TestCase): with urllib.request.urlopen(req, timeout=10) as r: raw = r.read() st = r.status - ct = r.headers.get("Content-Type", "") + rh = r.headers except urllib.error.HTTPError as e: raw = e.read() st = e.code - ct = e.headers.get("Content-Type", "") + rh = e.headers + ct = rh.get("Content-Type", "") if "json" in ct: - return st, json.loads(raw.decode()) - return st, raw + return st, json.loads(raw.decode()), rh + return st, raw, rh def wait_idle(self, timeout=10): deadline = time.time() + timeout @@ -161,6 +167,66 @@ class ServerTests(unittest.TestCase): st, d = self.call("GET", "/nope") self.assertEqual(st, 404) + def test_01b_state_is_conditional(self): + """The page polls; an unchanged state must cost a 304, and the + ETag must move as soon as the state does.""" + st, d, hdrs = self.call_h("GET", "/state") + self.assertEqual(st, 200) + etag = hdrs.get("ETag") + self.assertTrue(etag and etag.startswith('"'), "no ETag on /state") + st, _, hdrs2 = self.call_h("GET", "/state", headers={"If-None-Match": etag}) + self.assertEqual(st, 304) + self.assertEqual(hdrs2.get("ETag"), etag) + # a stale validator must not be honored + st, _, _ = self.call_h("GET", "/state", headers={"If-None-Match": '"stale"'}) + self.assertEqual(st, 200) + # and the validator moves with the state + self.call("POST", "/invalidate", {"reason": "etag check"}) + st, _, hdrs3 = self.call_h("GET", "/state", headers={"If-None-Match": etag}) + self.assertEqual(st, 200) + self.assertNotEqual(hdrs3.get("ETag"), etag) + + def test_01c_connection_is_kept_alive(self): + """A poll per second over a fresh TCP connection each time is + waste the board does not need to pay.""" + import http.client + c = http.client.HTTPConnection("127.0.0.1", self.port, timeout=10) + try: + for _ in range(3): + c.request("GET", "/state", headers={"Host": "127.0.0.1:%d" % self.port}) + r = c.getresponse() + r.read() + self.assertEqual(r.status, 200) + self.assertEqual(r.version, 11) + self.assertNotEqual((r.getheader("Connection") or "").lower(), "close") + self.assertIsNotNone(r.getheader("Content-Length")) + finally: + c.close() + + def test_01d_refused_post_does_not_poison_the_connection(self): + """A POST refused before its body is read leaves that body in the + socket. On a kept-alive connection the next read would take it for + a request line, so a refusal must end the connection instead.""" + import http.client + c = http.client.HTTPConnection("127.0.0.1", self.port, timeout=10) + try: + body = json.dumps({"test": "fake.pass"}).encode() + c.request("POST", "/start", body=body, + headers={"Host": "127.0.0.1:%d" % self.port, + "Content-Type": "application/json"}) # no token + r = c.getresponse() + payload = json.loads(r.read().decode()) + self.assertEqual(r.status, 403) + self.assertEqual(payload["error"], "authentication required") + self.assertEqual((r.getheader("Connection") or "").lower(), "close", + "an unread body was left on a connection kept alive") + finally: + c.close() + # the server is still healthy, and the body was never taken for a request + st, d = self.call("GET", "/state") + self.assertEqual(st, 200) + self.assertIsNone(d["running"]) + def test_02_run_pass_opens_campaign(self): st, d = self.call("GET", "/state") self.assertIsNone(d["campaign"])