vh/forgefirm
1
0
Fork 0
mirror of https://github.com/openglow-org/forgefirm.git synced 2026-09-28 01:01:12 -07:00
Code Issues Packages Projects Releases Wiki Activity

Make the acceptance page answer the operator, not the timer

Browse Source 
Presses on Start and Continue were being swallowed. Every poll rebuilt
the whole test table and the prompt buttons with innerHTML, and a button
destroyed between mousedown and mouseup raises no click event at all:
the press simply vanished. Measured on the page as it stood, a Start
button node was replaced 13 times in 40 seconds, and with a poll landing
mid-press 8 presses out of 8 were lost. Rows, prompt buttons and tool
entries are now built once and afterwards only updated in place through
setters that skip the write when the value has not changed; under the
same test 8 presses out of 8 land.

The page also felt slow because each poll re-read and re-parsed the whole
result log and recomputed all 42 domain fingerprints. A result record
carries its run log, so the file reaches megabytes over a campaign and
the poll cost grew with it. The log now parses each line once and reads
only the bytes appended since, and a fingerprint is memoized against the
manifest's content hash. On the same manifest, catalog and log, one
/state goes from 8.89 ms to 0.10 ms at 0.89 MB and from 41.79 ms to
0.23 ms at 10.62 MB, and no longer grows with the log.

Actions now report on the press instead of on the next poll: Start greys
every Start and marks the row, Continue and Abort grey themselves, and
each pulls the next poll forward. /state carries an ETag so an idle page
polls for a 304, the poll ticks faster during a run, and the connection
is kept alive rather than handshaking per request.

Keeping the connection alive exposed a hazard worth naming: a POST
refused before its body was read left that body in the socket, where the
next read took it for a request line. A refusal now ends the connection.

forgetest is a dev-only component and can never appear in a coverage map,
so this has no acceptance catalog consequence; the host tests carry the
proof, including one that fails if a per-poll innerHTML rebuild ever
comes back.
This commit is contained in:
ScottW514
2026-08-21 09:49:35 -04:00
parent eaa0b0741b
commit 474e14e0e8
7 changed files with 552 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files
forgetest/tests/test_responsiveness.py
+199
View File
@@ -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()
forgetest/tests/test_server.py
+70 -4
View File
@@ -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"])