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

Allow ignore reason #103

Merged
merged 2 commits into from
Jul 21, 2022
Merged

Allow ignore reason #103

merged 2 commits into from
Jul 21, 2022

Conversation

ijc
Copy link
Contributor

@ijc ijc commented Jul 20, 2022

Allow users to specify a reason why a test is inconclusive/ignored and translate into #[ignore = "Reason"]. This will be picked up by cargo test from 1.61.0 onward and displayed in the output. e.g.

#[test_case(() => inconclusive["reason but no comment"] ())]
#[test_case(() => inconclusive["reason and comment"] (); "test is not run")]
#[test_case(() => ignore["reason and comment"] (); "ignore keyword")]
fn descriptions(_: ()) {
    unreachable!()
}

Results in:

test descriptions::_expects_inconclusive_ ... ignored, reason but no comment
test descriptions::ignore_keyword ... ignored, reason and comment
test descriptions::test_is_not_run ... ignored, reason and comment

Fixes #102.

@luke-biel
Copy link
Collaborator

indexmap not keeping MSRV strikes again. I'm gonna look for fixes and get back to you. (You can run tests locally via scripts/test_all.sh. It's linux only though)

@ijc ijc force-pushed the allow-ignore-reason branch from e911d39 to e895761 Compare July 21, 2022 09:20
@ijc
Copy link
Contributor Author

ijc commented Jul 21, 2022

Thanks, I ran test_all.sh, nightly clippy had an unrelated complaint about implementing Eq -- I've added that commit.

In the end it failed for me with:

    Updating crates.io index
error: failed to download `hashbrown v0.12.3`

Caused by:
  unable to get packages from source

Caused by:
  failed to parse manifest at `/home/ianc/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/hashbrown-0.12.3/Cargo.toml`

Caused by:
  failed to parse the `edition` key

Caused by:
  this version of Cargo is older than the `2021` edition, and only supports `2015` and `2018` editions.

When testing with 1.49.0. Not sure if that is the same indexmap issue, I guess different MSRV issue?

@luke-biel
Copy link
Collaborator

luke-biel commented Jul 21, 2022

I've updated master with locks on problematic crates. You can merge in that branch and tests should pass now.

So hashbrown bumps MSRV as it should, in this case 0.11 -> 0.12 - expect breaking changes inbetween. Cargo won't bump this version on it's own. Problem, for second time, arises because indexmap bumped hashbrown version in 1.8 -> 1.9 transition. Thus implicit bump of MSRV in minor update.

I also found out that in the meantime serde_yaml did explicit MSRV bump in 0.8.25 -> 0.8.26 transition.

Overall, supporting older version of rust tend to be pain in the ***. But what can we do?

ijc added 2 commits July 21, 2022 10:39
It is sufficient to declare the type as the `kw::ignore` case does.
Since Rust 1.61.0 `cargo test` has included the reason string in the output.

The new syntax is optional, so `inconclusive` is still accepted, a reason may
be given inside optional brackets `inconclusive["some reason"]`. Parentheses
are not used to avoid ambiguity with the output/result.

A custom `impl Debug` is used to avoid injecting an unsightly
`inconclusivewithreason_litstr_token` into the test name by default.

Fixes: frondeus#102
@ijc ijc force-pushed the allow-ignore-reason branch from e895761 to 623b821 Compare July 21, 2022 10:54
@ijc
Copy link
Contributor Author

ijc commented Jul 21, 2022

Thanks, I've rebased and ./scripts/test_all.sh now passes

@luke-biel luke-biel merged commit 0757a9d into frondeus:master Jul 21, 2022
@luke-biel
Copy link
Collaborator

Will try to release it today or tomorrow

@ijc ijc deleted the allow-ignore-reason branch July 21, 2022 12:47
@ijc
Copy link
Contributor Author

ijc commented Jul 21, 2022

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support a string argument to "ignore" describing the reason
2 participants