nordisk/edda

crates/storage-git-gix/parked-gix-2/gunnar-store-gix/tests/swallowed_errors.rs

//! `S-030`: the failures this crate used to drop on the floor now say so.
//!
//! # Why a subscriber and not an eyeball
//!
//! Every fix under `S-030` replaces `let _ = …` with an `error!`, and "it emits
//! an error" is a claim about behaviour that only a subscriber can check. A
//! test that merely called the function and observed no panic would pass
//! against the swallow it is supposed to have removed — which is the shape the
//! whole finding is about.
//!
//! The capture is thread-local (`tracing::subscriber::with_default`), which is
//! sound here and would not be for a transfer: [`gunnar_store::drop_keep`] is
//! synchronous and runs on the caller's thread.

#![cfg(feature = "fs")]

use std::sync::{Arc, Mutex};

use tracing::subscriber::with_default;
use tracing_subscriber::layer::SubscriberExt;
use tracing_subscriber::Layer;

/// Every event seen, as `target` plus the rendered fields.
#[derive(Clone, Default)]
struct Capture(Arc<Mutex<Vec<(String, String)>>>);

impl Capture {
    fn events(&self) -> Vec<(String, String)> {
        self.0.lock().expect("the capture lock").clone()
    }
}

struct CaptureLayer(Capture);

impl<S: tracing::Subscriber> Layer<S> for CaptureLayer {
    fn on_event(&self, event: &tracing::Event<'_>, _: tracing_subscriber::layer::Context<'_, S>) {
        struct Render(String);
        impl tracing::field::Visit for Render {
            fn record_debug(&mut self, field: &tracing::field::Field, value: &dyn std::fmt::Debug) {
                self.0.push_str(&format!("{}={value:?} ", field.name()));
            }
        }
        let mut render = Render(String::new());
        event.record(&mut render);
        self.0
             .0
            .lock()
            .expect("the capture lock")
            .push((event.metadata().target().to_owned(), render.0));
    }
}

fn capturing<T>(body: impl FnOnce() -> T) -> (T, Vec<(String, String)>) {
    let capture = Capture::default();
    let subscriber = tracing_subscriber::registry().with(CaptureLayer(capture.clone()));
    let out = with_default(subscriber, body);
    (out, capture.events())
}

/// A `.keep` that will not go is **reported**, and the caller still returns.
///
/// The failure is forced by pointing `drop_keep` at a path inside a read-only
/// directory, which is a real `EACCES` from the real filesystem rather than a
/// stub — the point being that this is the error the production call sites
/// used to discard.
#[test]
fn a_keep_marker_that_cannot_be_removed_is_reported() {
    let dir = tempfile::tempdir().expect("temporary directory");
    let locked = dir.path().join("pack");
    std::fs::create_dir(&locked).expect("create");
    let keep = locked.join("pack-deadbeef.keep");
    std::fs::write(&keep, b"gunnar").expect("write the marker");

    let mut perms = std::fs::metadata(&locked).expect("metadata").permissions();
    perms.set_readonly(true);
    std::fs::set_permissions(&locked, perms).expect("make the directory read-only");

    let (_, events) = capturing(|| gunnar_store::drop_keep(&keep));

    // Put the permissions back before any assertion can leave the tempdir
    // undeletable.
    let mut perms = std::fs::metadata(&locked).expect("metadata").permissions();
    #[allow(clippy::permissions_set_readonly_false)]
    perms.set_readonly(false);
    std::fs::set_permissions(&locked, perms).expect("restore");

    assert!(
        keep.exists(),
        "the marker was removed after all, so this test did not force the failure \
         it is about"
    );
    let reported: Vec<_> = events
        .iter()
        .filter(|(target, fields)| target == "gunnar::store" && fields.contains("store.keep"))
        .collect();
    assert_eq!(
        reported.len(),
        1,
        "a .keep that could not be removed produced {} lines, not one. All events: {events:?}",
        reported.len()
    );
    assert!(
        reported[0].1.contains("pack-deadbeef.keep"),
        "the line does not name the marker: {:?}",
        reported[0]
    );
}

/// The half that stops the first test being satisfied by a `drop_keep` that
/// logs unconditionally — which would put an error line in an operator's
/// journal on every successful repack and every successful push.
#[test]
fn removing_a_keep_that_is_there_is_silent_and_actually_removes_it() {
    let dir = tempfile::tempdir().expect("temporary directory");
    let keep = dir.path().join("pack-c0ffee.keep");
    std::fs::write(&keep, b"gunnar").expect("write the marker");

    let (_, events) = capturing(|| gunnar_store::drop_keep(&keep));

    assert!(!keep.exists(), "the marker was not removed");
    assert!(
        events.is_empty(),
        "a successful removal was logged: {events:?}"
    );

    // Already gone is the outcome this function wants, not a failure.
    let (_, events) = capturing(|| gunnar_store::drop_keep(&keep));
    assert!(
        events.is_empty(),
        "removing an absent marker was reported as a failure: {events:?}"
    );
}

/// The target `drop_keep` writes to is the one an operator filters on.
///
/// `gunnar-store` cannot import `gunnar_core::observe::TARGET_STORE` — the
/// dependency runs the other way — so the string is restated in `repack.rs`,
/// and a restated constant that nothing compares is a constant that drifts.
#[test]
fn the_store_target_is_still_the_one_the_tree_agrees_on() {
    let dir = tempfile::tempdir().expect("temporary directory");
    let keep = dir.path().join("nested").join("pack-f00d.keep");
    // The parent does not exist, so removal fails with NotFound's sibling
    // rather than NotFound itself on some platforms; either way, if it does
    // not fail we say so instead of asserting on an empty capture.
    let (_, events) = capturing(|| gunnar_store::drop_keep(&keep));
    if events.is_empty() {
        // A missing parent yields NotFound, which is deliberately silent.
        // Nothing to assert; the read-only case above carries the target.
        return;
    }
    assert_eq!(events[0].0, "gunnar::store");
}