Cover the untested error branches in the deserialise paths

[FAZuH]

Part of the whole-repo test-suite audit (2026-09-30). Test error paths, not just happy paths: every branch that can fail needs a test.

What to build

Negative tests for every reject path in the three deserialise/parse surfaces, and a test that pins the on-disk state format.

Findings

crates/natmap/src/models.rs and crates/natmap/tests/model.rs contain no unwrap_err or is_err at all. Untested:

1. Unknown "proto":"xyz" deserialising into TransportProtocol (models.rs:30,119,248,268). The rejecting arm lives at crates/lab-lib/src/protocol.rs:34 and has only a doc test.
2. Missing required fields: DnatRequest.ext_ip/int_ip/ports, SnatRequest.ext_if, HairpinRequest.ext_ip, RProxyLocalConfig.template, ForwardRemoteConfig.port.
3. u16 out of range: ForwardLocalConfig.port, ForwardRemoteConfig.port, ext_ports entries, DockerAddMapRequest.host_port/container_port.
4. u32 out of range: PolicyRouteConfig.table and its request twin.
5. DaemonState (models.rs:338) state-file compatibility. policy_routes carries #[serde(default)] at :348; mapping, dnats, snats and hairpins do not. A state file written before policy_routes existed loads; a file missing any other top-level key hard-errors. This is a real on-disk format at /var/lib/natmap/state.json, and the forward path is untested.

crates/auto-discover/src/config.rs: the entire YAML surface is untested. Both existing tests (:458, :485) construct DiscoveryConfig structs directly; neither deserialises YAML. Untested: an unknown type: value, a missing node.name, a missing required type:, a missing template, and a port out of range.

src/cmd/dns_parser.rs:

• :52 caps[2].parse().unwrap_or(1): the TTL-parse-failure fallback to 1 is untested.
• :146 parts[0][1..].parse().unwrap_or(0): a non-numeric TLSA port label (_abc._tcp.mail) is untested.
• :170 TXT.captures_iter on an unterminated quote ("abc, no closing ") is untested. parse_txt_data_no_quotes (:501) covers zero quotes, not a broken one.

Acceptance criteria

• Every listed branch has a test that fails when the branch is removed
• DaemonState compatibility is pinned: a state file with and without policy_routes both load, and a file missing a required top-level key is rejected
• The auto_discover YAML surface has at least one test that goes through serde_yaml
• Test data is realistic (unicode, empty strings, boundary ports), per the guidelines
• Test names follow standards.md §3.6
• cargo test -p lab-ops_natmap, cargo test -p lab-ops_auto-discover, cargo test -p lab-ops --lib pass

Blocked by

None (can start immediately).

Also: docs/dev/testing.md:53 undercounts the suite by roughly 4× (it says 88 inline unit tests; the real figure is 421 tests). Correct it while in this file.

[FAZuH]

Landed on v0.2 as 8643493, merged 021c795. Closed by hand: a closing keyword only fires from commits reaching the default branch, and this is parked on v0.2.

Correction to the ticket's arithmetic

The ticket listed ~15 branches. 31 tests were added across three files — crates/natmap/tests/model.rs +16, crates/auto-discover/src/config.rs +11, src/cmd/dns_parser.rs +4.

The ticket's item 4, "PolicyRouteConfig.table and its request twin", has no twin: PolicyRouteRequest was deleted in 6877c6d, and api.rs:241 deserialises PolicyRouteConfig directly, so one test covers the path.

Every test was verified able to fail

Each mechanism was mutated and confirmed red, then restored:

• Unknown enum variant → 4 natmap *_rejects_unknown_proto + service_config_rejects_unknown_type
• Missing required field → 9 *_rejects_missing* tests
• u16 range → 5 range tests
• u32 range → policy_route_config_rejects_table_above_u32_max
• #[serde(default)] presence → daemon_state_loads_without_policy_routes
• TTL unwrap_or(1) → parse_zone_ttl_above_u32_max_falls_back_to_one
• TLSA unwrap_or(0) → parse_tlsa_name_non_numeric_port_falls_back_to_zero
• Unterminated quote → both parse_txt_data_unterminated_quote_*
• Positive path → 7 tests, added specifically to prove the accept-tests read real values rather than passing on a default

Two mutations had to be reworked: widening u16→u32 does not compile because callers do real arithmetic on those fields, so the range mechanism is verified through a wider-reading deserialiser that leaves the field type intact. Same for PolicyRouteConfig.table.

DaemonState compatibility pinned

A file with policy_routes loads; one without loads with it defaulting to empty; one missing dnats is rejected with missing field \dnats``.

A real bug found on the same path, filed as a new issue

daemon.rs:457 is serde_json::from_str(&data).unwrap_or_default(). In production a malformed state file does not error — it silently becomes DaemonState::default(), discarding every DNAT, SNAT, hairpin and port mapping the daemon had. persist() writes atomically via .tmp + rename, so the write side is safe; a half-written or externally-edited file on the read side looks like a clean restart. The new test pins the deserialiser, which is correct — the swallowing is at the call site. Logged rather than fixed, because changing it is a behaviour decision.

Also noted: adding #[serde(default)] to mapping/dnats/snats/hairpins would make truncated files load silently. Today only policy_routes has ever been added and it was defaulted, so no file on disk is at risk — but nothing prevents the next field from breaking every existing /var/lib/natmap/state.json.

docs/dev/testing.md:53

It said 88 inline unit tests. The real figure for inline unit tests — the four lib test binaries, as distinct from the integration and Docker targets — is 328, measured after a forced rebuild (57 + 157 + 80 + 34). The line now carries the command that produced it.

Flake, not caused by this change

registration::event_loop_logs_structured_fields failed once in two full-workspace runs at tests/auto_discover/mod.rs:139. Load-dependent: the box was at load average 8.66/11.50 and that run took 123.88s against ~31s isolated; the test passes 4/4 in isolation under the same load, and this change touches no Docker harness file. Tracked on #54.