Add ignore_if RFC
#3221
Add ignore_if RFC
#3221
Conversation
This RFC extends the `#[ignore]` annotation to accept named arguments and most importantly `if` parameter which specifies a predicate to ignore `#[test]` function based on a run time check.
|
There are three issues mixed here,
It may be good to separate these issues into different RFCs and figure out how to do. About runtime ignoring About the issue to ignore or test with conditions About the syntax Forget the implementation at first, #[test]
fn test_case(){
if env::var("SomeVar").is_none() {
ignore!("something we need is not here")
}
keep_test();
}It is really happy to see you here. 😄 ❤️ ⚙️ |
Those are the same question though? Regardless, this cannot be separated because answers to earlier questions influence subsequent decisions. It’s not possible to decide on syntax if it’s not decided whether ignoring
Regarding first question, it of course can be (and has been) argued that this feature should be left to test frameworks however the same could be said about about I had to go and look to find that apparently Catch doesn’t. In comparison GoogleTest supports it via Regarding second question, the flags are there so if we can we should care about them. Users would call test being ignored even if And this brings us to the third question, and with positive answer to the second we are now limited to an annotation. Now, whether it’s
And that syntax would stay. This RFC does not propose this syntax being removed. |
|
This feels very much like a library solution instead of a proper language solution. If I understand correctly, the goal is to be able to distinguish between ignored and passed tests at a glance, which the current attribute does not cover. IMHO, a more appropriate way to ignore at runtime would be to improve the existing So, something like: #[test]
fn test() -> Result<(), Ignored> {
if some_condition() {
Err(Ignored)
} else {
Ok(the_test())
}
} |
I’m not sure what you mean. Tests are handled by libtest so this is by definition a library change.
That’s the ‘Returning ignored status’ from the proposal and main issue with that is that |
nrc
added
A-test
|
Hi @clarfonthey, We always can do a lot of macro things in a framework to solve this issue, and if Rust can somehow change a little bit it will be better. Here is the definition of productivity of Rust on the website.
Such that we have However if Rust can not provide runtime ignoring feature, people will write code like following things mention in #68007, and no mater the realthing we want to test is passed or not, the test summary show on
This will somehow be away from the original productivity intention to provide useful error messages and top-notch tooling. |
|
I definitely didn't fully clarify what I meant, so, I'll be a bit more clear. What I mean by That said, the concept of marking tests as ignored at runtime isn't something that should be exclusively provided by crates, and I feel like the way we enable it should allow for a bunch of different use cases. The reason why I mention using a return value from the function for ignoring is it fits in with an existing feature (specifically, And, as a reminder, an To me, the ability to combine conditions with boolean operators is a must, and the function syntax just doesn't allow this in an easy way. A return value, although more verbose, would both support this easily in vanilla Rust, and support alternative syntaxes like the one proposed via proc macros. Adding a few examples of some cases that would make more sense as a return value: // "assume" macro
#[test]
fn do_something() -> Result<(), Ignored> {
assume!(target_supports_thing()); // if (target_supports_thing()) { return Err(Ignored); }
assume!(target_supports_other_thing());
Ok(do_test())
}
// multiple conditions
#[test]
fn do_other_thing() -> Result<(), Ignored> {
if target_supports_thing() && target_supports_other_thing() {
Ok(do_test())
} else {
Err(Ignored)
}
}
// try
#[test]
fn do_third_thing() -> Result<(), MyTestError> {
do_test_for(first_value())?;
do_test_for(second_value())
}
// (note: MyTestError implements some library trait that determines whether failure is ignore, or error) |
|
@clarfonthey could you spell out how return value based solutions would support That is, with #[test]
fn my_test() -> test::Result {
if target_supports_thing() {
return Err(test::Ignored);
}
// body of the test
Ok(())
}it seems that the body of the test wouldn’t get run even if user runs the test like $ cargo test -- --ignored my_test |
|
You're right, that solution alone wouldn't work with The only way I can see a return value-based solution working for that is if the return value implemented a trait like: where I'm not 100% familiar with what the API for custom test harnesses would look like (if it would exist) but presumably, That said, this kind of solution is definitely something that depends on the lofty goal of custom test harnesses being stable, which seems extremely far off. I don't think my arguments are worth counting against this particular implementation for that reason, unless someone comes up with a better idea or if one of my assumptions about the testing APIs is wrong. |
|
Hi there, @clarfonthey you are showing that the ignore a test case will write a lot of code and this may not align with the productive goal of Rust. We do really want the runtime to ignore feature in Rust right? By the way, there is another considering about the Currently, However, In that situiation, the attributes On the other hand, if Maybe it is good to use |
|
I should clarify, the implementation I suggested for a trait would only be an internal representation of ignorable tests; it would still require macros or attributes like the one you suggest. An Original: #[test]
fn test() {
ignore_if!(check_condition());
run_test();
}Expanded code: #[test]
fn test() {
if !include_ignored_tests() && condition {
send_ignore_signal();
return;
}
run_test();
}This could be via an argument to the test, maybe: #[test]
fn test(context: &TestContext) {
ignore_if!(context, check_condition());
run_test();
}Which would also work very well with RFC #2442: #[test]
fn test(context: &TestContext) {
context.ignore_if!(check_condition());
run_test();
}I guess that my main concern is that passing a function to an attribute "feels wrong" to me, but I'm not sure why. It would probably be fine to have even if another method of dynamic ignores were added in the future, since I'm not sure we want to commit to a |
No, this isn’t true for two reasons: First of all,
In this case I don’t think macros are necessary. Of course, the main question still remains: do we care about
For what it’s worth, there is a precedent in Serde which takes functions as arguments though it takes them as strings rather than plain paths. I’m honestly not sure if there’s a reason for that. |
I think paths are just syntactically invalid in that location, only literals are allowed: |
|
I was told that rust-lang/rust#57367 made paths valid and indeed it seems to be working with |
That's not true on the latest version of Rust; it's just that serde ensures very strong compatibility with old Rust version, where it's not allowed. |
|
Perhaps we're conflating two different features?
|
I'm not sure about that. I have tests that fail when my distro's package manager is running concurrently on the system. They are I want my CI to keep running every tests. If somehow the CI environment changes so that the tests would get runtime-ignored, I want a CI failure. In my normal workflow I'd probably stop passing
It's not just about impossible tests. My example with the package manager just make the test failure likely, and I could make the tests or code more resilient. A memory-hungry test could check the available memory before running, but be overridden by a developer who know his OS can take it. There are lots of motivations for runtime ignore, whatever |
|
Seeing more of the reasons someone might mark tests as ignored, I wonder if the best answer might simply be to make it easier to mark tests with arbitrary tags, then just allow filtering tests to allow or deny tests with certain flags. I know that when I was developing a lot of JavaScript, Mocha had the ability to run tests by a regexp match of the test name/group, which was both helpful for filtering out weird tests or checking individual tests from the entire suite without running everything. I'm mostly jumping to the extremely general solution since I think loads of people will have different use cases. If we had that, would "ignore" simply be another tag? Would |
|
I think that an enum return value is the simpler, more flexible approach. An attribute doesn't compose well, it introduces new syntax one has to remember, and the semantics aren't obvious; for example, a user might wonder whether a panicking predicate will cause the test to fail, or to be ignored. If we instead use established syntax (a simple I agree with @kornelski that overriding #[test]
fn my_test() -> TestResult {
if not_supported_on_this_platform() {
// always ignores the test
return TestResult::Ignore("not supported");
}
if im_feeling_lucky() {
// only ignores the test if CARGO_TEST_INCLUDE_IGNORED is not set
ignore_test!("I'm feeling lucky")?;
}
// do test...
TestResult::Success
}The About the |
|
Passing the "should ignored tests be included ?" information to the runtime seems like a good way to handle different reasons for ignored, I like it. However, it's not tied to the "attribute vs termination" debate, as Aloso's example could be written as: fn not_supported_on_this_platform(_include_ignored: bool) -> Option<String> {
/// answer regardless of include-ignored flag
(!is_linux()).then(|| "Only works on Linux")
}
fn low_ram(include_ignored: bool) -> Option<String> {
/// respect include-ignored flag
(ram() < 64 && !include_ignored).then(|| "You need 64G of RAM")
}
#[test]
#[ignore(if = not_supported_on_this_platform)]
fn my_linux_test() {
assert!(...);
}
#[ignore(if = low_ram)]
fn my_heavy_test() {
assert!(...);
}It seems to me that both are as versatile ? I prefer the attribute version as it seems less verbose/repetitive and closer to the existing functionality, but either would work. |
What you’re suggesting is introducing another kind of test result. And those need to be separate classification. If a test is ignored, We can argue whether
While I agree this is again a much bigger change to libtest. At the If I were to write testing framework from the start, I’d agree that I think
I dunno. Can it? It’s all software so anything is possible, but |
|
Prototype implementation: rust-lang/rust#96574 |
Co-authored-by: Hugo <[email protected]>
|
At the lowest level, I think tests that trigger an ignore should just return a I don't think The RFC currently mentions that this new return type would still have the need for the macro itself. The macro isn't a change inside rust/cargo: it can, via the The main barrier right now is for tests to somehow communicate to the user something other than success/fail; this new return type provides just that, and unlocked building all the other variants on top. |
Where would the
What you’ve cited doesn’t refer to need for a macro. Rather this is observation that with a custom return type, if you wanted to add conditional ignore to a test, you have to change return type of the test function. E.g. change |
The WhyNotHugo is referring to the lowest level, which could still be encapsulated within the (This is not meant to indicate that I'm for/against conditional tests respecting |
This RFC extends the
#[ignore]annotation to accept named argumentsand most importantly
ifparameter which specifies a predicate toignore
#[test]function based on a run time check.