400f0327af241239212cd3ae02b401ad8f9a7cee
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 James Mackay
|
7ed982801a |
test(adb-off audit): name the carve-outs, drive the paths the audit names (t25)
The t24 review's four observations on the t23 ADB-off audit. No production code
changes: button.py is untouched, deliberately (see O1).
O1 — THE PAIR ADB CARVE-OUT, ACCEPTED AND NAMED. The button's press builds its own
AdbMediaSessionPoller and connects without ever reading the entry's option. I
searched for a production route that reaches the press with ADB off and found
NONE, so no gate was added:
* `button.async_setup_entry` is the ONLY place that entity is created, and it
creates it only while the option is on (button.py:34-35);
* every options save runs through `NvidiaShieldOptionsFlow(OptionsFlowWithReload)`,
which reloads the entry and re-runs that setup — so turning ADB off takes the
button away (config_flow.py:1340-1353);
* the press is user-initiated in any case — the same class as the already
declared `test_connect` pairing probe.
It is now a NAMED carve-out in the audit's `CARVE_OUTS` table, with what it does,
why that is safe, and the assertions behind both. The measurement is DRIVEN rather
than quoted: the press is executed with the option OFF and ON and asserted to cost
the same 1 ctor / 1 connect / 1 disconnect, so the carve-out's substance is a test
result and not prose. A meta test checks every carve-out's named assertion really
exists, so a carve-out cannot point at a phantom. MUTATION-CHECKED: making
`async_setup_entry` add the entity unconditionally fails three of the new tests.
The intent button is explicitly NOT a carve-out (it can exist for a disabled entry
and refuses through the t23 accessor gate) — stated in the same entry.
O2 — THE AUDIT NOW EXECUTES WHAT IT NAMES. `_open_logcat_stream` and the logcat
ladder are driven both ways: two new rows in the parametrized `AUDITED_PATHS` table
(so the disabled case costs zero traffic) plus dedicated assertions — the open
RAISES "ADB is switched off" in a `pytest.raises(match=...)`, and the ladder
RETURNS at once with `_logcat_failures == []` and `_logcat_disabled is False`, i.e.
a local option is not booked as a device failure. The anti-overcorrection pin is
kept in the same class: with ADB ON, five real ladder failures DO auto-disable.
MUTATION-CHECKED: removing either t23 logcat gate fails three of these tests.
O3 — THE ROTTABLE FIGURE IS GONE. The statement_blocks bullet no longer states a
count ("currently 28" was stale within one unit of work); the band (~2%) is the
policy, the current walk lives in the header the sweep prints, and the comment now
says why no band carries a count. The relative_imports bullet lost its illustrative
figure for the same reason.
O4 — THE ONE WRITE IS NAMED AS A DEBT. The audit's module docstring now states
exactly what "nothing is cached" means: the single write a disabled path can still
make is `_app_label_failed.add(package)` in `_dump_label` — a retry DEBT, not an
answer. It is pinned by a test: with ADB off the debt is recorded while
`_app_labels` stays EMPTY and the lookup still answers None; with ADB back on the
probe runs, the label is cached and the debt is settled (the discard that the code
already does).
FAIL-CLOSED, CONFIRMED AND LOCATED. The harness change (`entry=None` now reads as
ADB off) matches production because both read the option the same way: the manager
returns "disabled" for a missing option, and `__init__.py` builds no poller for the
same missing option — asserted by two new tests (an entry with `options={}`, and a
manager with no entry at all) and by a source assertion on `__init__.py`'s own
`if not entry.options.get(CONF_ADB_ENABLED, False):`. No production path is
affected: production always constructs the manager WITH the entry, and for an entry
whose option is on nothing changed at all.
No regressions: 942 passed / 5 skipped (V1), 943 / 4 (V2) — the +19 are the new
tests. ruff clean, drift exit 0, the real-HA pattern collects and runs its 3 files.
The device side remains a scripted double: the claim proven is that nothing was
sent, not what firmware would do.
|
||
|
James Mackay
|
de1c34777d |
fix(adb): ADB-off is asked by every device path, not inferred from a missing poller (t23)
The t22 review's O1, and the same bug class as t21's F1 — so it gets the same
structural treatment instead of a note.
WHAT WAS MEASURED. With a poller injected into an entry whose own option said
`adb_enabled: False`, the installed-apps sweep sent 4 real shells (the reviewer;
3 on this test's double, which yields one launchable package) and a label probe
sent 1 more. Both gated on `self._poller is None` — and the poller's absence is a
property of the SETUP code (`_build_poller_factory` returns None when ADB is off),
which is precisely the indirect guarantee the t21 repair had already shown to be
insufficient. Production was unreachable only through that setup path.
THE FIX. `_adb_disabled()` is now asked directly by every path in the manager
that can reach the device, and `_adb_active()` (option ON *and* a poller) is the
shared predicate for the ones that also need a transport. The `available` property
deliberately keeps its transport meaning (diagnostics reads it as "ADB is
configured"); a new docstring says so, because overloading it is how the gate got
lost in the first place.
Gated paths, and what each asks now:
* intent scan — the option (t21, unchanged) AND the private shell site itself
* installed-apps sweep — the option first, then poller+connected
* installed-apps refresh (public entry) — returns the last table, starts nothing
* app-label lookup / drain / the ONE probe site (`_dump_label`) — the option; a
lookup returns None and queues NOTHING (no request, no task, no cache write)
* ADB text entry, media keys, bring-to-foreground, digit and enter keys — `_adb_active()`
* the heartbeat `_refresh` — the option: no wake probe, no media dump, no
running-apps poll, no foreground fallback, no label drain, no logcat ensure
* the watchdog — the option BEFORE the poller factory, so not even a factory can
build a transport; and `_watchdog_cycle` refuses a direct call too (a TCP
connect is device traffic)
* the logcat stream — the option at the ensure gate, the option in
`_open_logcat_stream`, and the ladder exits WITHOUT counting a failure (a local
option must not auto-disable the option for the session)
* the public `adb_poller` accessor — None while the option is off, which closes
the shell paths that live OUTSIDE this module (the `adb_command` service, the
screen-capture image and the intent shell) without reaching into them: all
three already refuse on None with their own message
* `test_connect` (the pairing probe) stays ungated on purpose: it runs BEFORE an
entry exists, and the setup menu only offers that step when the ADB option is
on (config_flow.py). `async_stop`/`_note_failure` disconnect calls are teardown,
not traffic.
PROOF, BY TRAFFIC. tests/test_adb_off_gates_every_device_path.py drives every
audited path — as a parametrized AUDIT table plus one full cycle on one manager —
with `adb_enabled: False` and a double that WOULD answer everything, and asserts
the poller's complete traffic list (shells, polls, commands, connects) is EMPTY,
plus that nothing was cached (no intent answer, no app table, no stamp, no queue,
no store field, no wake reading). It also proves the watchdog never builds a
poller and never connects, that a direct cycle does not connect either, that the
option is a CHECK and not a latch (the same manager resumes the sweep, the scan,
the label queue, the media key, the digit key and the accessor when the option
flips back on), and that ADB-off and no-poller stay told apart.
PRE-FIX PROOF: the same file against `git archive HEAD` FAILS 16 of 23 tests, and
the failures print the measured traffic — the sweep's 3 shells, the label probe's
`pm dump … | grep …`, `input text hello`, `media:play`, `foreground:<pkg>`,
`digit:5`, `enter`, and 4 polls per heartbeat cycle.
HARNESS TRUTHFULNESS. The suites' managers were built with `entry=None`, which
means ADB is OFF (fail closed, matching `__init__.py`). Those harnesses now pass a
realistic entry whose option says ADB is on — they model production instead of the
one combination the gates block. No production behaviour changed for an ADB-on
entry: 923 passed / 5 skipped, up from 900 by this file.
Floors re-derived from the new walk (1661 blocks → 1627, same 2% band): raised,
because that is what the re-measurement says. Drift exit 0. Strings files
byte-identical.
NOT VERIFIED HERE: the device side is a scripted double, so what is proven is that
NOTHING WAS SENT, not what firmware would do; no real Shield was involved.
|