S30: Mocks for everything; tests with sleep

S30Severity: Low

Tests that are brittle, slow, or test the mocks instead of the code.

Clippy: no lint finds this. Look for it in review.

What it looks like

The code under test: a function that sends reminders through a Mailer trait, and a function that starts a background export.

/// Starts the export in the background. The caller cannot tell when it is done.
pub fn start_export(rows: Vec<u64>, exported: Arc<AtomicUsize>) {
    thread::spawn(move || {
        for _row in rows {
            exported.fetch_add(1, Ordering::SeqCst);
        }
    });
}

The tests:

struct MockMailer {
    expected: Vec<(String, String)>,
    calls: RefCell<Vec<(String, String)>>,
}

impl MockMailer {
    fn expect_send(&mut self, to: &str, subject: &str) {
        self.expected.push((to.to_string(), subject.to_string()));
    }
    fn verify(&self) {
        assert_eq!(*self.calls.borrow(), self.expected, "unexpected calls");
    }
}

impl Mailer for MockMailer {
    fn send(&self, to: &str, subject: &str) {
        self.calls
            .borrow_mut()
            .push((to.to_string(), subject.to_string()));
    }
}

#[test]
fn sends_reminders() {
    let mut mailer = MockMailer {
        expected: vec![],
        calls: RefCell::new(vec![]),
    };
    mailer.expect_send("ada@example.com", "Invoice 7 is overdue"); // exact text, exact order
    let invoices = [Invoice {
        customer_email: "ada@example.com".into(),
        number: "7".into(),
        overdue: true,
    }];
    send_reminders(&invoices, &mailer);
    mailer.verify();
}

#[test]
fn exports_all_rows() {
    let exported = Arc::new(AtomicUsize::new(0));
    start_export(vec![1, 2, 3], Arc::clone(&exported));
    thread::sleep(Duration::from_millis(50)); // "long enough", on this machine, today
    assert_eq!(exported.load(Ordering::SeqCst), 3);
}

The mock records each call and compares it with a list of expected calls: the exact address, the exact subject text, in the exact order. It is Mockito’s verify, written by hand.
The sleep waits 50 ms and hopes that the background thread is done.

Why AI agents produce it

The Java habit. In Java projects, Mockito and verify are in almost every test class. An agent writes the same shape in Rust, by hand or with the mockall crate.
A sleep is the easy fix for a race. When a test fails because a thread is not done, sleep makes it pass on the agent’s machine.
The code gives no way to wait. start_export returns nothing, so the test has no handle to wait for. The real problem is in the code, not in the test.

Why it is a problem

The mock tests the implementation, not the result. Change the subject text, or send two reminders in another order, and the test fails, though the behavior is still correct.
The mock is often more code than the function. Reviewers then read and maintain the mock.
A sleep is slow. 50 ms in each of 200 tests is 10 seconds.
A sleep is flaky. On a busy CI runner, the thread may need more than 50 ms. cargo test runs tests in parallel threads by default, so the machine is busy. The test fails at random, and the team learns to ignore failures.

The better version

Return a JoinHandle, so the caller can wait:

/// Starts the export in the background, and returns a handle to wait for it.
pub fn start_export(rows: Vec<u64>) -> JoinHandle<usize> {
    thread::spawn(move || rows.len()) // the real work goes here
}

And test with a fake instead of a mock:

WorseA mock that checks every call and its exact text, and a sleep that hopes the thread is done.

struct MockMailer {
    expected: Vec<(String, String)>,
    calls: RefCell<Vec<(String, String)>>,
}

impl MockMailer {
    fn expect_send(&mut self, to: &str, subject: &str) {
        self.expected.push((to.to_string(), subject.to_string()));
    }
    fn verify(&self) {
        assert_eq!(*self.calls.borrow(), self.expected, "unexpected calls");
    }
}

impl Mailer for MockMailer {
    fn send(&self, to: &str, subject: &str) {
        self.calls
            .borrow_mut()
            .push((to.to_string(), subject.to_string()));
    }
}

#[test]
fn sends_reminders() {
    let mut mailer = MockMailer {
        expected: vec![],
        calls: RefCell::new(vec![]),
    };
    mailer.expect_send("ada@example.com", "Invoice 7 is overdue"); // exact text, exact order
    let invoices = [Invoice {
        customer_email: "ada@example.com".into(),
        number: "7".into(),
        overdue: true,
    }];
    send_reminders(&invoices, &mailer);
    mailer.verify();
}

#[test]
fn exports_all_rows() {
    let exported = Arc::new(AtomicUsize::new(0));
    start_export(vec![1, 2, 3], Arc::clone(&exported));
    thread::sleep(Duration::from_millis(50)); // "long enough", on this machine, today
    assert_eq!(exported.load(Ordering::SeqCst), 3);
}

BetterA small fake that records who got mail, a check of the outcome, and join() to wait for the thread.

/// A fake: a real, simple implementation that records what it sent.
#[derive(Default)]
struct SentMail(RefCell<Vec<String>>);

impl Mailer for SentMail {
    fn send(&self, to: &str, _subject: &str) {
        self.0.borrow_mut().push(to.to_string());
    }
}

#[test]
fn reminds_only_overdue_customers() {
    let invoice = |email: &str, overdue| Invoice {
        customer_email: email.into(),
        number: "7".into(),
        overdue,
    };
    let mail = SentMail::default();
    let sent = send_reminders(&[invoice("ada@x", true), invoice("bo@x", false)], &mail);
    assert_eq!(sent, 1);
    assert_eq!(*mail.0.borrow(), vec!["ada@x"]); // the outcome, not the call sequence
}

#[test]
fn exports_all_rows() {
    let handle = start_export(vec![1, 2, 3]);
    assert_eq!(handle.join().unwrap(), 3); // waits exactly as long as needed
}

A fake is a real, simple implementation of the trait. The test checks what matters: who got a reminder. It does not check the subject text or the call order.
join() waits exactly as long as needed. The test is as fast as the work, and never flaky.
Better still: no trait at all. If the function returned the list of reminders to send, the test would need no fake. Code that does the I/O at the edge is easier to test.

When it is acceptable

Mocks at a real boundary, such as an HTTP client or a payment service, when the test must check that a call happened: for example, that no payment is sent twice. Then mockall is a reasonable choice. Check that the expectations test the behavior, not every argument.
A sleep to test a timeout, where time is the behavior under test. Even then, a short timeout and a clear margin help. Async code with tokio can use paused time (tokio::time::pause, with the test-util feature) instead of real sleeps.

How to detect it

Search the tests for sleep. Each one needs a reason.
Look for expect_, verify, times(, or mockall in tests. Ask what behavior each expectation checks.
Look at the size: a test helper that is longer than the code under test is a signal.
A test that fails sometimes on CI and passes when you run it again is often a sleep or shared state between parallel tests.

What to tell your agent

Test the outcome, not the calls. Replace mocks that verify exact calls with
a small fake that records the result, and assert on that result. Remove
thread::sleep from tests: return a JoinHandle, a channel, or another way to
wait for the work, and wait on it. Do not add a trait only to mock it. Run
cargo test several times to check that the tests are stable.