eyre-rs / color-eyre

Custom hooks for colorful human oriented error reports via panics and the eyre crate
Other
958 stars 57 forks source link

panic in tests (I used master) #132

Open BppleMan opened 1 year ago

BppleMan commented 1 year ago

I have reviewed #78 and #144 But the problem is still not fixed

https://github.com/yaahc/color-eyre/blob/4a7b4d6988c6b0da5e04e29c9d6e10595b5dc302/tests/install.rs#L4-L7

I modified with

fn double_install_should_not_panic() {
    install().unwrap();
    install().unwrap();
    install().unwrap();
    assert!(install().is_err());
}

it's still panic

LeoniePhiline commented 8 months ago

I would consider this not a bug. Since https://github.com/eyre-rs/color-eyre/commit/48037df31155875c2c43b850445eda65979a5930 (https://github.com/eyre-rs/color-eyre/pull/114), install() does not panic, but returns a Result.

Previously, install itself panicked. This is fixed. However, if you unwrap the Result::Err, you cause a panic in your own code.

If you want to freely call install() multiple times, then use .ok() rather than .unwrap() to silence the resulting (#[must_use]) Result::Err by turning it into Option::None.

/// Does not panic.
fn double_install_should_not_panic() {
    install().ok();
    install().ok();
    install().ok();
    assert!(install().is_err());
}