Repository navigation
Table-drive the parse_port_mappings and iptables argv tests #45
Description
Part of the whole-repo test-suite audit (2026-09-30). This changes assertion quality, not just test count.
What to build
Two table-driven suites replacing 44 near-identical tests, and assertions strong enough that a wrong rule fails.
Findings
- 17
parse_port_mappingstests, ~27 lines each atcrates/lab-lib/src/docker.rs:512-971. Each builds a 10-line JSONContainerInspectResponse, calls the parser, and asserts one to three fields. A single table of(name, json, expected: Vec<DockerPortMapRequest>)is about 90 lines. Per the guidelines: use table-driven tests instead of a body full of near-identical setup; prefer DAMP over DRY in tests, but a shared JSON table here is readable rather than clever. - 27
args.contains(&token)argv assertions atcrates/natmap/src/iptables.rs:784-1122.dnat_args_unspecified_ip_omits_d_flag(:784) asserts only that the tokens-t,DNAT,8080,10.0.0.2:80appear somewhere in the vector;forward_accept_args_includes_ctn_ip_and_port(:839) is fourcontainscalls. A rule built in the wrong order, with the wrong-p, or with the port on the wrong side still passes. Replace withassert_eq!(args, vec![...]), one table per builder (build_dnat_rule_args,build_forward_accept_args,build_masquerade_args,build_output_dnat_args, thehairpin_*builders,cmd_for).
Acceptance criteria
- Each suite is one table-driven test (or one per builder), not a body containing a loop
- Every iptables argv assertion is an exact-vector comparison, not a substring check
- Deliberately corrupting a builder (swap an argument order, drop a flag) makes the new tests fail. Demonstrate this.
- Per the guidelines: no
if,for,whileormatchinside a test body - Test names follow
standards.md§3.6:<module_or_function>_<scenario> -
cargo test -p lab-ops_lab-libandcargo test -p lab-ops_natmap --libpass
Blocked by
- Collapse the Config/Request type pairs in natmap models #42
- Extract the repeated natmap api and table helpers #43
Estimated: -44 tests, -550 lines.
Activity
test: exact-vector argv assertions, and tables for the flat repeats
Merge test/45-table-driven into v0.2: exact-vector argv assertions (#45)
FAZuH commentedon Oct 1, 2026
Landed on
v0.2asde1a891, merged0db8006. Closed by hand: a closing keyword only fires from commits reaching the default branch.The defect, stated plainly
dnat_args_unspecified_ip_omits_d_flagasserted only that-t,DNAT,8080and10.0.0.2:80appear somewhere in the argument vector.forward_accept_args_includes_ctn_ip_and_portwas fourcontainscalls. A rule built in the wrong order, with the wrong-p, or with the port on the wrong side passed both. Every argv assertion is now an exact-vector comparison against a literal.Eleven corruptions, each shown red and reverted
build_dnat_rule_args(swap--dport/-j) ·build_forward_accept_args(drop--dport) ·build_masquerade_args(--dportbefore-p) ·build_output_dnat_args(--dporttakes the container port) ·build_loopback_masq_args(ipv6 loopback src) ·build_static_dnat_prerouting_args(-iafter-d) ·build_static_dnat_forward_args(FORWARD → DOCKER-USER) ·build_snat_args(-stakes ext_ip) ·build_hairpin_prerouting_args(--to-destinationtakes ext_ip) ·build_hairpin_postrouting_args(lan_cidr ignored) ·cmd_for(families swapped) · plus four variants ofparse_port_mappings.A real gap the work found
The
output_dnatcase inherited9090/9090from the original — host port and container port identical — so swapping them was invisible and the corruption passed. The fixture is now9090/9443and the corruption fails correctly. The old test could not have caught that either.The line count is -187, not the -550 the ticket estimated
parse_port_mappingsiptablesargvcmd_forassertsThe iptables file grew 35 lines and that is the correct outcome. A
containscheck is about six lines; an exact 22-token vector is the case plus a literal that rustfmt wraps. Reaching −550 would have meant going back to substring assertions, which is the defect this ticket exists to remove.The ticket's own counts were also off: it said 17
parse_port_mappingstests where there are 21, and estimated −44 tests where the real merge is −36.Coverage held
47 cases before, 47 after, enumerated name by name. The 21 docker rows map 1:1. The iptables 25 rows cover 25 builder tests, and the two
cmd_fortests merged into one test with twoassert_eq!, losing no assertion.Shape, and why
A table plus a helper that owns the loop, not a
macro_rules!macro: the macro route puts the case name nowhere in the failure message without a#[test]per row, while the helper printscase: <name>. Verified mechanically — noif,for,whileormatchtoken survives in any test body once string literals are stripped.Split, not tabled, across the 11 argv tests. They do not share a row shape: five builders take a
DockerPortMap, three aDnatConfig, two aHairpinConfig, one aSnatConfig, and two returnOptionwhere eight returnVec. One table would need a per-builder input constructor column and a sentinel for theOptionbuilders — the same failure mode #46 refused earlier in this batch.Verification
Every hunk sits inside a
#[cfg(test)]module:docker.rshunks run 378..767 against#[cfg(test)]at 288,iptables.rs730..1101 against one at 720. The only two survivingcontains(&are in productiondelete_by_commentmatching a rule's comment string, which is not an argv assertion../dev.sh test: 427 passed, 0 failed, 10 targets, exit 0, at loadavg 1.61/1.58 before and 2.66/1.85 after. Down 36 from 463, which is exactly 48 tests merged into 12.