The PR description mentioned this test but it was missing from the
diff, leaving backup_has_real_upstream_windows untested. Mirrors the
shape of macos_backup_real_upstream_detection: empty map → false,
all-loopback (127.0.0.1, ::1, 0.0.0.0) → false, one real entry
alongside loopback → true.
Also relax the cfg gate on backup_has_real_upstream_windows from
cfg(windows) to cfg(any(windows, test)) so the test compiles
cross-platform, matching how backup_has_real_upstream_macos and
the resolv_conf helpers are gated.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The test inlined the 3-line filter block from install_windows rather
than calling a production helper, so it was testing stdlib Vec::retain
+ is_loopback_or_stub — both already covered elsewhere. Deleting it
removes a test that would silently pass even if install_windows stopped
filtering altogether.
The predicate logic for macOS-shaped backups stays covered by
macos_backup_real_upstream_detection (same inner Vec<String> type).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Remove duplicated read+deserialize boilerplate shared by install_macos
and install_windows. The two call sites each had an identical 4-line
chain of read_to_string().ok().and_then(serde_json::from_str).ok() —
collapse into a single generic helper load_backup<T>().
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
CI caught `clippy::trim_split_whitespace` on Rust 1.94: `split_whitespace()`
already skips leading/trailing whitespace, so `.trim()` first is redundant.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Post-review simplifications on the stale-backup fix:
- Extract iter_nameservers(&str) helper used by both parse_resolv_conf
and resolv_conf_has_real_upstream. Eliminates the duplicated
line-by-line nameserver parsing (findings from reuse review).
- install_linux: reuse the already-read resolv.conf content via
std::fs::write instead of a second read via std::fs::copy.
- install_macos / install_windows: flatten the conditional eprintln
pattern — always print a blank line, conditionally print the save
message. Equivalent output, less branching.
Net −12 lines. All 130 tests still pass, clippy clean.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The install flow previously captured current system DNS servers
verbatim into the backup file. If numa was already installed, current
DNS was 127.0.0.1, so the "backup" recorded 127.0.0.1 as the "original"
— making a subsequent uninstall a no-op self-reference.
Reproduced 2026-04-08 during v0.10.0 brew dogfood: after
`sudo numa uninstall; sudo /opt/homebrew/bin/numa install`,
`sudo numa uninstall` printed `restored DNS for "Wi-Fi" -> 127.0.0.1`
because the brew binary's install step had overwritten the backup with
the already-stub state.
Fix (all three platforms):
- macOS/Windows: if the existing backup already contains at least one
non-loopback/non-stub upstream, preserve it as-is. If writing a fresh
backup, filter loopback/stub addresses first so a capture from
already-numa-managed state isn't self-referential.
- Linux (resolv.conf fallback path): detect numa-managed or all-loopback
resolv.conf content and skip the file copy in that case; preserve an
existing useful backup rather than overwriting it. systemd-resolved
path is unaffected (uses a drop-in, no backup file).
Adds three unit tests for the predicates: macOS HashMap detection,
Windows interface filter, and resolv.conf parsing (real upstream,
self-referential, numa-marker, systemd stub, mixed).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
