diff --git a/forgetest/forgetest/fixture.py b/forgetest/forgetest/fixture.py index fa8597b..ee907b1 100644 --- a/forgetest/forgetest/fixture.py +++ b/forgetest/forgetest/fixture.py @@ -43,7 +43,14 @@ PULSE_WAIT_S = 1.5 # a pulse in progress ends within the firmware's class FixtureError(Exception): - pass + """A request the fixture did not carry out as asked. `sent` is True + when the failure came after the request went out (no reply, or a + reply cut off), so the box may have acted on it; a refusal, a failed + lookup and a failed connect leave it False.""" + + def __init__(self, *args, sent=False): + super().__init__(*args) + self.sent = sent # ---------------------------------------------------------------- mDNS @@ -271,11 +278,20 @@ class Fixture: body = {} return e.code, body except (urllib.error.URLError, socket.timeout, OSError) as e: + # urllib raises a failed connect or send as a URLError; a + # timeout or a reset after the request went out comes bare, and + # the box may have acted on the request + sent = not isinstance(e, urllib.error.URLError) if retry and not self.ip_override: - # the address may have moved: one fresh lookup, one retry - self.address(refresh=True) - return self._request(method, path, body, retry=False) - raise FixtureError("fixture %s: %s %s: %s" % (self.where(), method, path, e)) + # the address may have moved: one fresh lookup, one retry, + # and a failure of either still says the first one went out + try: + self.address(refresh=True) + return self._request(method, path, body, retry=False) + except FixtureError as again: + again.sent = again.sent or sent + raise + raise FixtureError("fixture %s: %s %s: %s" % (self.where(), method, path, e), sent=sent) def status(self): st, body = self._request("GET", "/") diff --git a/forgetest/forgetest/runner.py b/forgetest/forgetest/runner.py index 75f6db5..d23028d 100644 --- a/forgetest/forgetest/runner.py +++ b/forgetest/forgetest/runner.py @@ -485,7 +485,7 @@ class Context: except _fixture.FixtureError as e: rec["fixture_error"] = str(e) if self._press_taken(rec, e): - return True + return rec["by"] == "fixture" self.log("ARM press: fixture failed (%s) - asking the operator", e) self.notice(text) self._clear_when_taken() @@ -496,16 +496,23 @@ class Context: def _press_taken(self, rec, err): """After the box reported a failed press of a lit button: True when - the button is out, so the controller took a press (the box pressed - and its reply was lost). The press is recorded as the box's, and - no notice goes up, because it would ask for a second press, and a - press during the job pauses it. A button still lit, or unreadable, - is False: the operator is asked.""" + the button is out, so the controller took a press. No notice goes + up then, because it would ask for a second press, and a press during + the job pauses it. The press is recorded as the box's only when its + request went out and the failure came after it (err.sent: the box + pressed and its reply was lost). After a refusal, a failed lookup + or a failed connect the box made no press, and the record stays as + it was: the press was a person's. A button still lit, or + unreadable, is False: the operator is asked.""" if hw.button_lit() is not False: return False - rec["by"] = "fixture" - self.log("ARM press: the fixture reported a failure (%s), and the button is out: the press " - "was taken; no notice", err) + if getattr(err, "sent", False): + rec["by"] = "fixture" + self.log("ARM press: the fixture reported a failure (%s), and the button is out: the press " + "was taken; no notice", err) + else: + self.log("ARM press: the fixture made no press (%s), and the button is out: a press was " + "taken all the same; no notice", err) return True def _clear_when_taken(self, lit_timeout=ARM_LIT_WAIT_S): diff --git a/forgetest/tests/test_fixture.py b/forgetest/tests/test_fixture.py index a0ad57a..ec2f1bd 100644 --- a/forgetest/tests/test_fixture.py +++ b/forgetest/tests/test_fixture.py @@ -40,6 +40,8 @@ class FakeFixture: self.button_enabled = button_enabled self.calls = [] self.presses = 0 # presses the device performed + self.reply_delay = 0.0 # seconds between a press and its reply + self.replied = 0 # delayed replies the device is done with (sent or lost) srv = self class H(BaseHTTPRequestHandler): @@ -95,6 +97,14 @@ class FakeFixture: threading.Timer(0.05, lambda: srv.state.__setitem__("button", "idle")).start() s = self._state() s["pulse_ms"] = 200 + if srv.reply_delay: + time.sleep(srv.reply_delay) # the reply comes after the client gave up + try: + self._json(200, s) + except OSError: + pass + srv.replied += 1 + return None return self._json(200, s) if self.path == "/release": srv.state = {"lid": "closed", "interlock": "closed", "button": "idle"} @@ -109,6 +119,11 @@ class FakeFixture: self.httpd.shutdown() self.httpd.server_close() + def wait_replied(self, n, timeout=5.0): + deadline = time.time() + timeout + while self.replied < n and time.time() < deadline: + time.sleep(0.02) + class ConfigTests(unittest.TestCase): def setUp(self): @@ -155,11 +170,11 @@ class ClientTests(unittest.TestCase): self.resolved.append(hostname) return "127.0.0.1" - def client(self, **over): + def client(self, timeout=3.0, resolver=None, **over): cfg = {"hostname": "forgefixture", "key": KEY, "channels": ["lid", "interlock", "button"], "port": self.dev.port} cfg.update(over) - return fx.Fixture(cfg, resolver=self.resolver) + return fx.Fixture(cfg, timeout=timeout, resolver=resolver or self.resolver) def test_status_covers_act_release(self): f = self.client() @@ -196,6 +211,35 @@ class ClientTests(unittest.TestCase): f.act("button", "press") self.assertIn("jumper", str(cm.exception)) + def test_a_failure_after_the_request_went_out_says_so(self): + # The press made and its reply lost (it comes after the client's + # timeout): sent, one press. A refusal, a failed lookup and a + # refused connect: not sent, no press. The one retry keeps it. + self.dev.reply_delay = 0.6 + with self.assertRaises(fx.FixtureError) as cm: + self.client(timeout=0.2, ip="127.0.0.1").act("button", "press") + self.assertTrue(cm.exception.sent) + self.dev.wait_replied(1) + self.assertEqual(self.dev.presses, 1) + answers = ["127.0.0.1"] # the refresh lookup after the timeout finds nothing + with self.assertRaises(fx.FixtureError) as cm: + self.client(timeout=0.2, resolver=lambda h: answers.pop(0) if answers else None).act("button", "press") + self.assertTrue(cm.exception.sent) + self.dev.wait_replied(2) + self.assertEqual(self.dev.presses, 2) + self.dev.reply_delay = 0.0 + self.dev.button_enabled = False + with self.assertRaises(fx.FixtureError) as cm: + self.client(ip="127.0.0.1").act("button", "press") + self.assertFalse(cm.exception.sent) + with self.assertRaises(fx.FixtureError) as cm: + self.client(resolver=lambda h: None).act("button", "press") + self.assertFalse(cm.exception.sent) + with self.assertRaises(fx.FixtureError) as cm: + self.client(timeout=0.5, ip="127.0.0.1", port=1).act("button", "press") + self.assertFalse(cm.exception.sent) + self.assertEqual(self.dev.presses, 2) + def test_a_wrong_key_is_refused(self): f = self.client(key="wrong") with self.assertRaises(fx.FixtureError) as cm: @@ -631,7 +675,7 @@ class RoutingTests(unittest.TestCase): # press was taken, and no notice asks for a second one. def pressed_then_failed(channel, state, ms=None): lit["v"] = False # the controller takes the press - raise fx.FixtureError("timed out") + raise fx.FixtureError("timed out", sent=True) self.stub.act = pressed_then_failed lit["v"] = True self.assertTrue(ctx.press_now()) @@ -645,6 +689,25 @@ class RoutingTests(unittest.TestCase): time.sleep(0.1) self.assertEqual(ctx.evidence["actions"][-1]["by"], "fixture") self.assertIsNone(run.notice) + # A refusal (or a request that never reached the box) while the + # button went out: a press at the machine was taken, so no notice + # goes up either, and the record does not give it to the box. + def refused_while_pressed(channel, state, ms=None): + lit["v"] = False # a person's press is taken + raise fx.FixtureError("button press -> 409 button disabled: the enable jumper is out") + self.stub.act = refused_while_pressed + lit["v"] = True + self.assertFalse(ctx.press_now()) + self.assertIsNone(run.notice) + self.assertEqual(ctx.evidence["actions"][-1]["by"], "operator") + lit["v"] = True + self.assertTrue(ctx.arm_press()) + deadline = time.time() + 2 + while "fixture_error" not in ctx.evidence["actions"][-1] and time.time() < deadline: + time.sleep(0.02) + time.sleep(0.1) + self.assertEqual(ctx.evidence["actions"][-1]["by"], "operator") + self.assertIsNone(run.notice) finally: runner_mod.hw.button_lit = saved