Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic in tests (I used master) #132

Open
BppleMan opened this issue Jul 4, 2023 · 1 comment
Open

panic in tests (I used master) #132

BppleMan opened this issue Jul 4, 2023 · 1 comment
Labels
bug Something isn't working

Comments

@BppleMan
Copy link

BppleMan commented Jul 4, 2023

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

@thenorili thenorili added the bug Something isn't working label Nov 20, 2023
@LeoniePhiline
Copy link

I would consider this not a bug. Since 48037df (#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());
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants