test(transport): close the vacuous and uncovered branches

Six tests that asserted nothing, or asserted less than they claimed.

`a_bind_fault_still_fails_the_start` returned early whenever the socket
*could* be opened, so it was vacuous as root and on any developer machine with
a group-readable /dev/bpf* — the fail-fast path it is named for went unchecked
exactly where someone was most likely to run it. It now asserts in both
halves, and the privileged half is worth more than the fix: a present,
bindable interface binding inline is the ordinary case on a booted router, and
no other unit test reaches it, because every other one here names an interface
that does not exist. The bind-success path had no unit coverage at all.

`an_interface_with_no_addresses_is_still_present` returned early when the
fixture was absent — no fixture, pass. It still has to skip on a machine with
no address-less interface, so the guard is the runner declaring that it has
fixtures: CI now sets FIPS_TEST_REQUIRE_FIXTURES beside
FIPS_TEST_ADDRLESS_IFACE, and the test fails rather than skips if the fixture
step is ever removed or renamed. Its `let _ = interface_carrier(...)` is now
asserted too: if presence and carrier ever collapsed into one read, a
carrier-less bridge would report absent and the whole IFF_UP-not-IFF_RUNNING
decision would be silently undone.

`policy_labels` checked `Required.as_str()` and not `Optional.as_str()`, so a
swapped pair would paint every expected interface as the tolerated kind and
stay green. `Presence::as_str` had no test at all — `binding` was never
observed by anything, anywhere.

Three binder branches had no coverage: a transport restarting (the second
`start_async` clearing the previous run's stop flag — only a second *stop* was
tested, so a transport that could never restart passed everything), the
episode clock being restamped at start rather than at construction, and the
refused-edge retry actually delivering. The last one matters most: the
existing test filled the channel and dropped the receiver, so a slot that
captured an edge and never re-sent it would pass while health sat on a stale
level forever.

And the hardware-change boundary `record_bind` returns, which the neighbour
flush hangs off: false on a first bind (or every clean start would drop a
cache it had just built) and true once, not stickily, on a MAC change.

Each new test was verified against the defect it guards — comment out the
`shutdown.store(false)`, the `mark_starting()`, or the seeded `unpublished`,
and the corresponding test goes red while the rest stay green.

.github/workflows/ci.yml

@@ -303,6 +303,9 @@ jobs:
             exit 1
           fi
           echo "FIPS_TEST_ADDRLESS_IFACE=fips-probe0" >> "$GITHUB_ENV"
+          # Declare that this runner has fixtures, so a test that depends on
+          # one fails when the fixture is missing instead of skipping silently.
+          echo "FIPS_TEST_REQUIRE_FIXTURES=1" >> "$GITHUB_ENV"
 
       - name: Install Rust toolchain
         uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1
@@ -443,6 +446,9 @@ jobs:
             exit 1
           fi
           echo "FIPS_TEST_ADDRLESS_IFACE=fips-probe0" >> "$GITHUB_ENV"
+          # Declare that this runner has fixtures, so a test that depends on
+          # one fails when the fixture is missing instead of skipping silently.
+          echo "FIPS_TEST_REQUIRE_FIXTURES=1" >> "$GITHUB_ENV"
 
       - name: Install Rust toolchain
         uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1

src/transport/ethernet/mod.rs

@@ -1651,10 +1651,13 @@ mod tests {
             io::interface_present(loopback),
             "loopback must be present for this test to mean anything"
         );
-        if PacketSocket::open(loopback, 0x2121).is_ok() {
-            // This host can open the socket, so there is no fault to observe.
-            return;
-        }
+        // Whether the socket opens splits the test into two halves, and both
+        // assert. Returning early on the privileged host — which is what this
+        // used to do — made the test vacuous as root and on any developer
+        // machine with a group-readable /dev/bpf*, so the fail-fast path it is
+        // named for went unchecked exactly where someone was most likely to be
+        // running it.
+        let can_open = PacketSocket::open(loopback, 0x2121).is_ok();
 
         let config = EthernetConfig {
             interface: loopback.to_string(),
@@ -1672,6 +1675,29 @@ mod tests {
         let (tx, _rx) = super::super::packet_channel(8);
         let mut eth = EthernetTransport::new(TransportId::new(9), None, config, tx);
 
+        if can_open {
+            // The privileged half. A present, bindable interface binds inline
+            // and reports itself bound before `start_async` returns — which is
+            // the ordinary case on a booted router, and which no other unit
+            // test reaches: every other one here uses an interface that does
+            // not exist, so the bind-success path has no unit coverage at all
+            // without this branch.
+            eth.start_async()
+                .await
+                .expect("a present, bindable interface must start");
+            assert_eq!(
+                eth.presence(),
+                Presence::Present,
+                "a bind that succeeded must leave the transport present"
+            );
+            assert!(
+                eth.binding.socket().is_some(),
+                "a present transport must hold its socket"
+            );
+            eth.stop_async().await.expect("stop");
+            return;
+        }
+
         let err = eth
             .start_async()
             .await
@@ -1853,7 +1879,18 @@ mod tests {
     /// perfectly well-addressed interface without anyone noticing.
     #[test]
     fn an_interface_with_no_addresses_is_still_present() {
+        // A silent skip is how this test spent its life green without ever
+        // running: no fixture, early return, pass. It still has to skip on a
+        // developer machine that has no address-less interface, so the guard
+        // is the runner declaring that it *does* — if the fixture step is
+        // removed or renamed, this fails instead of quietly covering nothing.
         let Ok(iface) = std::env::var("FIPS_TEST_ADDRLESS_IFACE") else {
+            assert!(
+                std::env::var_os("FIPS_TEST_REQUIRE_FIXTURES").is_none(),
+                "this runner sets FIPS_TEST_REQUIRE_FIXTURES but not \
+                 FIPS_TEST_ADDRLESS_IFACE: the fixture step did not run, and \
+                 the musl/glibc getifaddrs contract this pins went unchecked"
+            );
             return;
         };
         assert!(
@@ -1864,7 +1901,14 @@ mod tests {
         );
         // It has no carrier either — a dummy device is up but not running —
         // which pins that presence and carrier really are separate reads.
-        let _ = io::interface_carrier(&iface);
+        // Asserted rather than discarded: if these two ever collapsed into the
+        // same read, a carrier-less bridge would report absent and the whole
+        // IFF_UP-not-IFF_RUNNING decision would be silently undone.
+        assert!(
+            !io::interface_carrier(&iface),
+            "{iface} is up with no carrier, so presence and carrier must \
+             disagree here — if they agree, they are the same read"
+        );
         // And it resolves to an index, which is what a bind would attach to.
         assert!(io::interface_index(&iface).is_some());
     }
@@ -1999,6 +2043,90 @@ mod tests {
         assert!(eth.binding.socket().is_none());
     }
 
+    #[tokio::test]
+    async fn a_restarted_transport_starts_its_binder_again() {
+        // `start_async` clears the stop flag so a restart is not immediately
+        // undone by the previous run's shutdown. Nothing tested the second
+        // start at all: `the_binder_stops_with_the_transport` only asserts a
+        // second *stop* errors, so a transport that could never be restarted
+        // would have passed everything here.
+        let (mut eth, _rx) = absent_transport(false);
+
+        eth.start_async().await.expect("first start");
+        eth.stop_async().await.expect("stop");
+        assert!(eth.shutdown.load(Ordering::SeqCst), "stop raises the flag");
+
+        eth.start_async()
+            .await
+            .expect("a stopped transport restarts");
+        assert!(
+            !eth.shutdown.load(Ordering::SeqCst),
+            "the restart must clear the previous run's stop, or the new binder \
+             tears its own binding down on its first pass"
+        );
+        assert_eq!(eth.state(), TransportState::Up);
+
+        eth.stop_async().await.expect("stop again");
+    }
+
+    #[tokio::test]
+    async fn the_absence_deadline_is_measured_from_the_start() {
+        // `PresenceState::new` stamps the episode clock at construction, but
+        // construction and `start_async` need not be adjacent — config load and
+        // supervisor staging sit between them. Without the restamp a transport
+        // staged for longer than the window reports sustained absence on its
+        // very first binder tick, having given the interface no bring-up window
+        // at all, which is the one thing the window exists to provide.
+        let (mut eth, _rx) = absent_transport(false);
+
+        // Stand in for a slow bring-up by ageing the clock past the deadline.
+        std::thread::sleep(Duration::from_millis(20));
+        let staged_for = eth.presence.since();
+
+        eth.start_async().await.expect("start");
+        assert!(
+            eth.presence.since() < staged_for,
+            "the episode clock must restart at start, not run from whenever \
+             the object happened to be constructed"
+        );
+
+        eth.stop_async().await.expect("stop");
+    }
+
+    #[tokio::test]
+    async fn a_refused_presence_edge_is_delivered_once_there_is_room() {
+        // The retry slot, which nothing followed through. The existing test
+        // fills the channel and asserts the binder keeps running, then drops
+        // the receiver — so a slot that captured the edge and never re-sent it
+        // would pass, and health would sit on a stale level forever.
+        let (mut eth, _rx) = absent_transport(false);
+        let (tx, mut presence_rx) = tokio::sync::mpsc::channel(1);
+        eth.set_presence_tx(tx.clone());
+
+        // Occupy the only slot, so the start edge is refused on its way out.
+        tx.try_send(TransportPresence {
+            transport_id: TransportId::new(99),
+            present: true,
+            health_relevant: true,
+        })
+        .expect("the one slot");
+
+        eth.start_async().await.expect("start");
+
+        // Drain the squatter. The binder now has room on its next pass.
+        let squatter = presence_rx.recv().await.expect("squatter");
+        assert_eq!(squatter.transport_id, TransportId::new(99));
+
+        let edge = tokio::time::timeout(Duration::from_secs(5), presence_rx.recv())
+            .await
+            .expect("the refused edge must be retried, not dropped")
+            .expect("channel open");
+        assert_eq!(edge.transport_id, TransportId::new(1));
+        assert!(!edge.present, "the retried edge is the absence it refused");
+
+        eth.stop_async().await.expect("stop");
+    }
+
     #[tokio::test]
     async fn a_full_presence_channel_does_not_block_the_binder() {
         // The health channel must not be able to deadlock the machine whose

src/transport/ethernet/presence.rs

@@ -768,5 +768,72 @@ mod tests {
         assert!(AbsencePolicy::Optional.is_optional());
         assert!(!AbsencePolicy::Required.is_optional());
         assert_eq!(AbsencePolicy::Required.as_str(), "required");
+        // Both labels, not just one. `show_transports` renders this string and
+        // fipstop's severity split keys on it, so a swapped pair would paint
+        // every expected interface as the tolerated kind and vice versa —
+        // while a test that checks only `Required` stays green through it.
+        assert_eq!(AbsencePolicy::Optional.as_str(), "optional");
+    }
+
+    #[test]
+    fn a_first_bind_is_not_a_hardware_change() {
+        // The boundary the flush hangs off. `record_bind` returns "different
+        // hardware", and on the very first bind there is no previous MAC to
+        // differ from — so it must answer false, or every clean start would
+        // drop a neighbour cache it had just built and log a hardware swap
+        // that never happened.
+        let state = PresenceState::new();
+        assert!(
+            !state.record_bind([1, 2, 3, 4, 5, 6]),
+            "the first bind has nothing to differ from"
+        );
+        assert_eq!(state.binds(), 1);
+        assert_eq!(state.presence(), Presence::Present);
+    }
+
+    #[test]
+    fn a_rebind_on_new_hardware_reports_the_change_once() {
+        // And it reports the change once, not on every subsequent bind: the
+        // caller drops its cached neighbours on a `true`, so a sticky answer
+        // would flush the cache on every rebind forever.
+        let state = PresenceState::new();
+        state.record_bind([1, 2, 3, 4, 5, 6]);
+
+        assert!(
+            state.record_bind([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]),
+            "a name returning on a different MAC is different hardware"
+        );
+        assert!(
+            !state.record_bind([0xaa, 0xbb, 0xcc, 0xdd, 0xee, 0xff]),
+            "the same hardware rebinding is not a change"
+        );
+        assert_eq!(state.binds(), 3);
+    }
+
+    #[test]
+    fn every_presence_label_is_distinct_and_round_trips() {
+        // The labels are the operator-facing vocabulary — `show_transports`
+        // emits them and the fipstop State column renders them — and
+        // `Presence::as_str` had no test at all, so `binding` in particular was
+        // never observed by anything.
+        use std::collections::HashSet;
+        let all = [Presence::Absent, Presence::Binding, Presence::Present];
+        let labels: HashSet<&str> = all.iter().map(|p| p.as_str()).collect();
+        assert_eq!(labels.len(), 3, "each phase needs its own label");
+        assert!(labels.contains("binding"));
+
+        for phase in all {
+            assert_eq!(
+                Presence::from_u8(phase.as_u8()),
+                phase,
+                "{phase} must survive the atomic round trip the state uses"
+            );
+            assert_eq!(phase.to_string(), phase.as_str(), "Display must agree");
+        }
+
+        // Anything outside the enum reads as absent rather than panicking: the
+        // byte comes out of an AtomicU8 that a torn write could leave at any
+        // value, and the safe answer there is "not bound".
+        assert_eq!(Presence::from_u8(99), Presence::Absent);
     }
 }