Add optional ignore message #27
Conversation
| is_ignored: bool, | ||
| ignored_message: Option<String>, |
There was a problem hiding this comment.
I would prefer if we made the state is_ignored: false, ignored_message: Some(_) impossible to represent here. The state should not happen. So I'd suggest:
| is_ignored: bool, | |
| ignored_message: Option<String>, | |
| ignored: Ignored, |
And add a new enum:
enum Ignored {
No,
Yes { msg: Option<String> },
}| /// or was ran anyway with `--ignored` flag. | ||
| /// | ||
| /// Default value is `None`, which means no specific message is included. | ||
| pub fn with_ignored_message(self, ignored_message: Option<String>) -> Self { |
There was a problem hiding this comment.
I thought a bit about this API. Ideally the API should also make it impossible to configure bogus states. As stated in your docs, the proposed API allows .with_ignored_flag(true).with_ignored_message(Some("hi")). That's not optimal.
I'm not 100% sure yet, but how about instead of with_ignored_message, we add this?
pub fn with_ignored(ignored: impl Into<Ignored>) -> Self I.e. make the Ignored enum public and then add the following impls:
impl From<bool> for Ignored { ... }
impl<T: Into<String>> From<T> for Ignored { ... }
Then users could call:
.with_ignored(false)
.with_ignored(true)
.with_ignored("my message")
The last case is a bit implicit, but its meaning (#[ignored = "my message"]) should still be very clear.
If we do it like that I would #[deprecate] with_ignored_flag as it's not necessary anymore and longer than .with_ignored(bool).
| @@ -27,38 +27,40 @@ fn tests() -> Vec<Trial> { | |||
| Trial::bench("green", |_| Err("was poisoned".into())).with_kind("kiwi"), | |||
| Trial::bench("purple", |_| Ok(meas(100, 5))).with_ignored_flag(true), | |||
| Trial::bench("cyan", |_| Err("not creative enough".into())).with_ignored_flag(true), | |||
| Trial::bench("magenta", |_| Ok(meas(5, 10))).with_ignored_flag(true).with_ignored_message(Some("flaky test!".to_string())), | |||
There was a problem hiding this comment.
Can you pick another color? :D One that is at most as long as the other trial names. That way, the diffs below are way smaller.
|
As I haven't heard back, I will close this PR. If you want to take another stab at this, feel free to reopen or open a new PR. |
This checks off one of the tasks on #13