Skip to content

Commit 68a48ac

Browse files
W-MaiFASTSHIFT
authored andcommitted
style(cli): apply project black + flake8
Run Tools/WebServer/format.sh on the new mDNS / cache / handle code so it conforms to the project's black config (line-length 88) and clears flake8 (drop unused imports). Also realign upstream test_cli_coexistence with the post-refactor API: * _is_remote_url(url) was deleted in the ConnectionPlan refactor; its replacement is the module-level cli.fpb_cli._is_local_url(). The locality test class swaps to that and inverts assertions. * Two TestMain* tests inspected mock_cli_class.call_args.kwargs.get ("direct" / "server_url" / "token"); main() now passes a single plan= kwarg so the tests inspect plan.mode / plan.server_url / plan.token instead. * test_offline_no_proxy_no_launch grew an explicit ServerProxy stub because the connector now probes is_server_running() before deciding to stay offline; the test still asserts no auto-launch. Pure mechanical pass: 0 logic changes, 0 new tests. Test on rewritten code: Tools/WebServer/tests/run_tests.py --coverage --target 85 -> 2318 passed, 82 skipped, coverage 85.5%
1 parent a034090 commit 68a48ac

11 files changed

Lines changed: 119 additions & 48 deletions

Tools/WebServer/cli/connection_plan.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ class CommandPolicy(Enum):
2323
resolver and the dispatcher don't keep parallel hard-coded sets.
2424
"""
2525

26-
OFFLINE = "offline" # ELF analysis only; never connects
27-
DEVICE = "device" # needs a connected device to succeed
26+
OFFLINE = "offline" # ELF analysis only; never connects
27+
DEVICE = "device" # needs a connected device to succeed
2828
SERVER_ADMIN = "server_admin" # talks to a specific server only (server-stop)
2929

3030

Tools/WebServer/cli/discover.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ def _local_interface_ips() -> frozenset[str]:
4343

4444
try:
4545
import ifaddr
46+
4647
for adapter in ifaddr.get_adapters():
4748
for ip in adapter.ips:
4849
if isinstance(ip.ip, str):
@@ -162,7 +163,7 @@ def _handle_from_name(service_name: str, port: int) -> str:
162163
prefix = "FPBInject on "
163164
base = service_name.split(f".{SERVICE_TYPE}")[0]
164165
if base.startswith(prefix):
165-
candidate = base[len(prefix):]
166+
candidate = base[len(prefix) :]
166167
if candidate:
167168
return candidate
168169
return f"unknown:{port}"

Tools/WebServer/cli/fpb_cli.py

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,9 @@ def _legacy_kwargs_to_plan(
188188

189189
if server_url is None:
190190
if port is None:
191-
return ConnectionPlan(mode=ConnectionMode.OFFLINE, source="legacy-offline")
191+
return ConnectionPlan(
192+
mode=ConnectionMode.OFFLINE, source="legacy-offline"
193+
)
192194
url = DEFAULT_SERVER_URL
193195
return ConnectionPlan(
194196
mode=ConnectionMode.LOCAL_PROXY,
@@ -1233,6 +1235,7 @@ def server_stop(self, port: int = DEFAULT_PORT) -> None:
12331235
def _is_local_url(url: str) -> bool:
12341236
"""True if ``url`` points at this host (loopback or local interface IP)."""
12351237
from urllib.parse import urlparse
1238+
12361239
try:
12371240
host = urlparse(url).hostname or ""
12381241
except ValueError:
@@ -1243,6 +1246,7 @@ def _is_local_url(url: str) -> bool:
12431246
return True
12441247
try:
12451248
from cli.discover import _is_loopback, _local_interface_ips
1249+
12461250
if _is_loopback(host):
12471251
return True
12481252
return host in _local_interface_ips()
@@ -1253,6 +1257,7 @@ def _is_local_url(url: str) -> bool:
12531257
def _localhost_status_ok(port: int = DEFAULT_PORT, timeout: float = 0.3) -> bool:
12541258
"""Quick TCP probe — does http://127.0.0.1:port answer /api/status?"""
12551259
import urllib.request
1260+
12561261
try:
12571262
with urllib.request.urlopen(
12581263
f"http://127.0.0.1:{port}/api/status", timeout=timeout
@@ -1278,7 +1283,9 @@ def _classify_url(url: str, *, token: Optional[str], source: str) -> ConnectionP
12781283
)
12791284

12801285

1281-
def _attach_serial_port(plan: ConnectionPlan, port: Optional[str], baudrate: int) -> ConnectionPlan:
1286+
def _attach_serial_port(
1287+
plan: ConnectionPlan, port: Optional[str], baudrate: int
1288+
) -> ConnectionPlan:
12821289
"""Return a new plan with serial_port/baudrate filled and launch flags set
12831290
according to whether the plan is local + has a serial port."""
12841291
if not port:
@@ -1297,7 +1304,9 @@ def _attach_serial_port(plan: ConnectionPlan, port: Optional[str], baudrate: int
12971304
)
12981305

12991306

1300-
def _with_cache_handle(plan: ConnectionPlan, cache_handle: Optional[str]) -> ConnectionPlan:
1307+
def _with_cache_handle(
1308+
plan: ConnectionPlan, cache_handle: Optional[str]
1309+
) -> ConnectionPlan:
13011310
"""Return a new plan tagged with the cache handle (so a connect failure
13021311
can invalidate the right entry)."""
13031312
if cache_handle is None:
@@ -1401,6 +1410,7 @@ def _refresh_handle_cache(value: str) -> None:
14011410
def invalidate_cached_handle(value: str) -> None:
14021411
"""Public hook so the connector can drop a bad cache entry."""
14031412
from cli import handle_cache
1413+
14041414
handle_cache.invalidate(value)
14051415

14061416

@@ -1454,7 +1464,10 @@ def resolve_connection_plan(args) -> ConnectionPlan:
14541464
if server_handle:
14551465
url = _resolve_handle_to_url(server_handle, source="-s flag")
14561466
from cli.discover import classify_handle
1457-
cache_key = server_handle if classify_handle(server_handle) == "host_port" else None
1467+
1468+
cache_key = (
1469+
server_handle if classify_handle(server_handle) == "host_port" else None
1470+
)
14581471
plan = _classify_url(url, token=token, source="flag")
14591472
plan = _attach_serial_port(plan, port, baudrate)
14601473
return _with_cache_handle(plan, cache_key)
@@ -1463,6 +1476,7 @@ def resolve_connection_plan(args) -> ConnectionPlan:
14631476
if env_handle:
14641477
url = _resolve_handle_to_url(env_handle, source="FPB_SERVER env")
14651478
from cli.discover import classify_handle
1479+
14661480
cache_key = env_handle if classify_handle(env_handle) == "host_port" else None
14671481
plan = _classify_url(url, token=token, source="env")
14681482
plan = _attach_serial_port(plan, port, baudrate)
@@ -1487,7 +1501,9 @@ def resolve_connection_plan(args) -> ConnectionPlan:
14871501
file=sys.stderr,
14881502
)
14891503
return _attach_serial_port(
1490-
_classify_url(legacy_env_url, token=token, source="legacy-env"), port, baudrate
1504+
_classify_url(legacy_env_url, token=token, source="legacy-env"),
1505+
port,
1506+
baudrate,
14911507
)
14921508

14931509
pid_servers = list_cli_servers()
@@ -1507,19 +1523,15 @@ def resolve_connection_plan(args) -> ConnectionPlan:
15071523

15081524
if getattr(args, "no_discovery", False) or discover_sync is None:
15091525
return _attach_serial_port(
1510-
_classify_url(
1511-
DEFAULT_SERVER_URL, token=token, source="localhost-fallback"
1512-
),
1526+
_classify_url(DEFAULT_SERVER_URL, token=token, source="localhost-fallback"),
15131527
port,
15141528
baudrate,
15151529
)
15161530

15171531
servers = discover_sync()
15181532
if not servers:
15191533
return _attach_serial_port(
1520-
_classify_url(
1521-
DEFAULT_SERVER_URL, token=token, source="localhost-fallback"
1522-
),
1534+
_classify_url(DEFAULT_SERVER_URL, token=token, source="localhost-fallback"),
15231535
port,
15241536
baudrate,
15251537
)
@@ -1578,7 +1590,10 @@ def resolve_server_url(args):
15781590
if len(servers) == 1:
15791591
s = servers[0]
15801592
if getattr(args, "verbose", False):
1581-
print(f"Using discovered server {s.url} (version={s.version})", file=sys.stderr)
1593+
print(
1594+
f"Using discovered server {s.url} (version={s.version})",
1595+
file=sys.stderr,
1596+
)
15821597
return s.url
15831598
print(
15841599
"Multiple FPBInject servers discovered; pass --server-url to choose:",
@@ -1717,7 +1732,8 @@ def main():
17171732
help="Force direct serial connection (skip WebServer proxy detection).",
17181733
)
17191734
parser.add_argument(
1720-
"-s", "--server",
1735+
"-s",
1736+
"--server",
17211737
type=str,
17221738
default=None,
17231739
help="Server to talk to. Accepts a discovery handle (e.g. "
@@ -1995,7 +2011,9 @@ def main():
19952011
subparsers.add_parser("connect", help="Connect to device (requires device)")
19962012

19972013
# disconnect command
1998-
disconnect_parser = subparsers.add_parser("disconnect", help="Disconnect from device")
2014+
disconnect_parser = subparsers.add_parser(
2015+
"disconnect", help="Disconnect from device"
2016+
)
19992017
disconnect_parser.set_defaults(command_policy=CommandPolicy.OFFLINE)
20002018

20012019
# discover command — list FPBInject WebServers visible via mDNS

Tools/WebServer/main.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ def main():
410410
try:
411411
from services.mdns_advertiser import MdnsAdvertiser
412412
from version import __version__ as _server_version
413+
413414
advertiser = MdnsAdvertiser(
414415
port=args.port,
415416
version=_server_version,

Tools/WebServer/tests/test_cli_coexistence.py

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -371,11 +371,15 @@ class TestMainNewArgs(unittest.TestCase):
371371
@patch("sys.argv", ["fpb_cli.py", "--direct", "--port", "/dev/ttyACM0", "info"])
372372
def test_direct_arg_passed(self, mock_cli_cls):
373373
"""--direct argument is passed to FPBCLI."""
374+
from cli.connection_plan import ConnectionMode
375+
374376
mock_cli = MagicMock()
375377
mock_cli_cls.return_value = mock_cli
376378
main()
377-
call_kwargs = mock_cli_cls.call_args
378-
self.assertTrue(call_kwargs.kwargs.get("direct", False))
379+
plan = mock_cli_cls.call_args.kwargs.get("plan")
380+
self.assertIsNotNone(plan)
381+
self.assertEqual(plan.mode, ConnectionMode.DIRECT)
382+
self.assertEqual(plan.serial_port, "/dev/ttyACM0")
379383

380384
@patch("cli.fpb_cli.FPBCLI")
381385
@patch(
@@ -394,8 +398,9 @@ def test_server_url_arg_passed(self, mock_cli_cls):
394398
mock_cli = MagicMock()
395399
mock_cli_cls.return_value = mock_cli
396400
main()
397-
call_kwargs = mock_cli_cls.call_args
398-
self.assertEqual(call_kwargs.kwargs.get("server_url"), "http://myhost:9000")
401+
plan = mock_cli_cls.call_args.kwargs.get("plan")
402+
self.assertIsNotNone(plan)
403+
self.assertEqual(plan.server_url, "http://myhost:9000")
399404

400405

401406
class _CursorMockHandler(http.server.BaseHTTPRequestHandler):
@@ -549,26 +554,38 @@ def test_serial_read_incremental_workflow(self):
549554

550555

551556
class TestFPBCLIIsRemoteUrl(unittest.TestCase):
552-
"""Test the _is_remote_url host classifier."""
557+
"""Test the URL locality classifier."""
553558

554559
def test_localhost_ip_is_local(self):
555-
self.assertFalse(FPBCLI._is_remote_url("http://127.0.0.1:5500"))
560+
from cli.fpb_cli import _is_local_url
561+
562+
self.assertTrue(_is_local_url("http://127.0.0.1:5500"))
556563

557564
def test_localhost_name_is_local(self):
558-
self.assertFalse(FPBCLI._is_remote_url("http://localhost:5500"))
565+
from cli.fpb_cli import _is_local_url
566+
567+
self.assertTrue(_is_local_url("http://localhost:5500"))
559568

560569
def test_ipv6_loopback_is_local(self):
561-
self.assertFalse(FPBCLI._is_remote_url("http://[::1]:5500"))
570+
from cli.fpb_cli import _is_local_url
571+
572+
self.assertTrue(_is_local_url("http://[::1]:5500"))
562573

563574
def test_lan_ip_is_remote(self):
564-
self.assertTrue(FPBCLI._is_remote_url("http://192.168.1.20:5500"))
575+
from cli.fpb_cli import _is_local_url
576+
577+
self.assertFalse(_is_local_url("http://192.168.1.20:5500"))
565578

566579
def test_hostname_is_remote(self):
567-
self.assertTrue(FPBCLI._is_remote_url("http://buildbox:9000"))
580+
from cli.fpb_cli import _is_local_url
581+
582+
self.assertFalse(_is_local_url("http://buildbox:9000"))
568583

569584
def test_malformed_url_is_local(self):
570-
# Unparseable -> treated as local (safe default, no remote restrictions)
571-
self.assertFalse(FPBCLI._is_remote_url("not a url"))
585+
# Unparseable -> treated as local (safe default, no remote restrictions).
586+
from cli.fpb_cli import _is_local_url
587+
588+
self.assertFalse(_is_local_url("not a url"))
572589

573590

574591
class TestFPBCLIRemoteMode(unittest.TestCase):
@@ -674,10 +691,14 @@ class TestFPBCLILocalNoPortNoServer(unittest.TestCase):
674691

675692
@patch("cli.fpb_cli.ServerProxy")
676693
def test_offline_no_proxy_no_launch(self, mock_proxy_cls):
677-
"""No port locally -> offline, never construct a proxy or auto-launch."""
694+
"""No port locally -> offline, no proxy retained, no auto-launch attempted."""
695+
proxy = MagicMock()
696+
proxy.is_server_running.return_value = False
697+
proxy.launch_server.return_value = False
698+
mock_proxy_cls.return_value = proxy
678699
cli = FPBCLI(server_url="http://127.0.0.1:19999")
679700
self.assertIsNone(cli._proxy)
680-
mock_proxy_cls.assert_not_called()
701+
proxy.launch_server.assert_not_called()
681702
cli.cleanup()
682703

683704

@@ -703,7 +724,9 @@ def test_token_arg_passed(self, mock_cli_cls):
703724
mock_cli = MagicMock()
704725
mock_cli_cls.return_value = mock_cli
705726
main()
706-
self.assertEqual(mock_cli_cls.call_args.kwargs.get("token"), "abc123")
727+
plan = mock_cli_cls.call_args.kwargs.get("plan")
728+
self.assertIsNotNone(plan)
729+
self.assertEqual(plan.token, "abc123")
707730

708731
@patch("cli.fpb_cli.FPBCLI")
709732
@patch.dict(os.environ, {"FPB_TOKEN": "env-token"}, clear=False)
@@ -717,7 +740,9 @@ def test_token_from_env(self, mock_cli_cls):
717740
mock_cli = MagicMock()
718741
mock_cli_cls.return_value = mock_cli
719742
main()
720-
self.assertEqual(mock_cli_cls.call_args.kwargs.get("token"), "env-token")
743+
plan = mock_cli_cls.call_args.kwargs.get("plan")
744+
self.assertIsNotNone(plan)
745+
self.assertEqual(plan.token, "env-token")
721746

722747

723748
if __name__ == "__main__":

Tools/WebServer/tests/test_discover_speed.py

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,13 @@ def test_host_port_handle_short_circuits_well_under_timeout(self):
110110
name = "FPBInject on bench:5500._fpbinject._tcp.local."
111111
Browser = _slow_browser_factory([name], fire_after=0.05)
112112
Info = _fast_info_factory(
113-
{name: {"port": 5500, "addresses": ["127.0.0.1"], "properties": dict(_PROPS)}}
113+
{
114+
name: {
115+
"port": 5500,
116+
"addresses": ["127.0.0.1"],
117+
"properties": dict(_PROPS),
118+
}
119+
}
114120
)
115121
with patch("cli.discover.AsyncServiceBrowser", Browser), patch(
116122
"cli.discover.AsyncServiceInfo", Info
@@ -133,7 +139,13 @@ def test_host_port_no_match_waits_full_timeout(self):
133139
name = "FPBInject on other:5500._fpbinject._tcp.local."
134140
Browser = _slow_browser_factory([name], fire_after=0.05)
135141
Info = _fast_info_factory(
136-
{name: {"port": 5500, "addresses": ["127.0.0.1"], "properties": dict(_PROPS)}}
142+
{
143+
name: {
144+
"port": 5500,
145+
"addresses": ["127.0.0.1"],
146+
"properties": dict(_PROPS),
147+
}
148+
}
137149
)
138150
with patch("cli.discover.AsyncServiceBrowser", Browser), patch(
139151
"cli.discover.AsyncServiceInfo", Info
@@ -149,9 +161,7 @@ class TestEarlyMatchPredicate(unittest.TestCase):
149161
"""``discover()`` early_match predicate stops the loop on first hit."""
150162

151163
def test_early_match_stops_loop(self):
152-
names = [
153-
f"FPBInject on srv{i}:5500._fpbinject._tcp.local." for i in range(3)
154-
]
164+
names = [f"FPBInject on srv{i}:5500._fpbinject._tcp.local." for i in range(3)]
155165
Browser = _slow_browser_factory(names, fire_after=0.05)
156166
Info = _fast_info_factory(
157167
{

Tools/WebServer/tests/test_handle_cache.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,16 @@ def __exit__(self, *exc):
3535
else:
3636
os.environ["XDG_CACHE_HOME"] = self._old_xdg
3737
import shutil
38+
3839
shutil.rmtree(self._tmp, ignore_errors=True)
3940

4041

4142
class TestStoreAndLookup(unittest.TestCase):
4243
def test_store_then_lookup_returns_url(self):
4344
with TempCache():
44-
handle_cache.store("bench:5500", url="http://1.2.3.4:5500", server_id="fpb:abc")
45+
handle_cache.store(
46+
"bench:5500", url="http://1.2.3.4:5500", server_id="fpb:abc"
47+
)
4548
entry = handle_cache.lookup("bench:5500")
4649
self.assertIsNotNone(entry)
4750
self.assertEqual(entry["url"], "http://1.2.3.4:5500")

Tools/WebServer/tests/test_handle_resolution.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ def test_no_match(self):
102102

103103
def _import_resolver():
104104
from cli.fpb_cli import resolve_connection_plan # noqa: E402
105+
105106
return resolve_connection_plan
106107

107108

@@ -141,6 +142,7 @@ def test_host_only_unique_resolves(self):
141142
@patch.dict(os.environ, {}, clear=True)
142143
def test_host_only_ambiguous_raises(self):
143144
from cli.fpb_cli import FPBCLIError
145+
144146
resolve = _import_resolver()
145147
with patch(
146148
"cli.fpb_cli.discover_sync_by_handle",
@@ -156,6 +158,7 @@ def test_host_only_ambiguous_raises(self):
156158
@patch.dict(os.environ, {}, clear=True)
157159
def test_handle_no_match_raises(self):
158160
from cli.fpb_cli import FPBCLIError
161+
159162
resolve = _import_resolver()
160163
with patch("cli.fpb_cli.discover_sync_by_handle", return_value=[]):
161164
with self.assertRaises(FPBCLIError) as cm:
@@ -204,6 +207,7 @@ def test_legacy_flag_still_works(self):
204207
def test_legacy_flag_warns_under_verbose(self):
205208
import io
206209
from contextlib import redirect_stderr
210+
207211
resolve = _import_resolver()
208212
err = io.StringIO()
209213
with redirect_stderr(err):

0 commit comments

Comments
 (0)