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

The decision is made at compile time rather than run time #18

Closed
mina86 opened this issue Jan 7, 2022 · 11 comments · Fixed by #63
Closed

The decision is made at compile time rather than run time #18

mina86 opened this issue Jan 7, 2022 · 11 comments · Fixed by #63
Labels
pending Issue can not be solved now

Comments

@mina86
Copy link

mina86 commented Jan 7, 2022

$ cat src/lib.rs
#[test_with::env(SOME_VAR)]
fn test() {
    panic!("WUT");
}
$ cargo test --tests
   Compiling a v0.1.0 (/tmp/a)
    Finished test [unoptimized] target(s) in 0.23s
     Running unittests (target/debug/deps/a-07e254e8b610defe)

running 1 test
test test ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s

$ : This should fail but it does not:
$ SOME_VAR=should_panic cargo test --tests
    Finished test [unoptimized] target(s) in 0.03s
     Running unittests (target/debug/deps/a-07e254e8b610defe)

running 1 test
test test ... ignored

test result: ok. 0 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.00s

$ touch src/lib.rs
$ SOME_VAR=should_panic cargo test --tests
   Compiling a v0.1.0 (/tmp/a)
    Finished test [unoptimized] target(s) in 0.19s
     Running unittests (target/debug/deps/a-07e254e8b610defe)

running 1 test
test test ... FAILED

failures:

---- test stdout ----
thread 'test' panicked at 'WUT', src/lib.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass '--lib'

$ : This should succeed but fails:
$ cargo test --tests
    Finished test [unoptimized] target(s) in 0.03s
     Running unittests (target/debug/deps/a-07e254e8b610defe)

running 1 test
test test ... FAILED

failures:

---- test stdout ----
thread 'test' panicked at 'WUT', src/lib.rs:3:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    test

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.00s

error: test failed, to rerun pass '--lib'
@yanganto
Copy link
Owner

yanganto commented Jan 8, 2022

Thanks for opening this issue, the readme lacks information on this.

Now, The Readme is updated to help people know how to use, and why this can not be solved now.
There is a related issue already mentioned in rust-lang/rust, and also it also listed in the readme.

Currently, this repo can provide build-time ignoring check but not runtime ignoring check, and helps users build and test a larger application with much more integration.
I will try to figure out if there is an RFC for this else I will try to add a new one for this.

By the way, there is also an RFC related to this crate, you may check it out.
rust-lang/rfcs#3217

@yanganto yanganto added the pending Issue can not be solved now label Jan 8, 2022
@mina86
Copy link
Author

mina86 commented Jan 8, 2022

Ah, sorry. I’ve actually skipped the README and didn’t notice that this was already mentioned there.

@mina86 mina86 closed this as completed Jan 8, 2022
@yanganto
Copy link
Owner

yanganto commented Jan 8, 2022

Don't worry.
The readme is not well documented, so the user can not in the same page as me.
I reopen this to remind me to keep an eye on the latest status of Rust.

Once we can ignore the test case in the test case itself, I will refactor this.

@yanganto yanganto reopened this Jan 8, 2022
@mina86
Copy link
Author

mina86 commented Jan 8, 2022

I will try to figure out if there is an RFC for this else I will try to add a new one for this.

I’ve actually run into #68007 (which is in fact how I’ve discovered your crate) but then the linked pre-RFC has been dead since October and there seems to be nothing happening regarding this feature.

I’ve been actually thinking about creating RFC myself if there’s no progress on the process.

@yanganto
Copy link
Owner

yanganto commented Jan 8, 2022

The RFC happens only someone keeps pushing and taking care of it, If you create an RFC on this, please let me know, and I am willing to help and learn more from this. Thanks.

@mina86
Copy link
Author

mina86 commented Jan 17, 2022

If you create an RFC on this, please let me know, and I am willing to help and learn more from this.

I’ve written a draft and plan to submit it later in a few days: https://github.com/mina86/rfcs/blob/master/text/0000-ignore-if.md

@yanganto
Copy link
Owner

It may be good just call ignore!("The message") in the test case then the test case is ignored. Besides, ignore! is a clear and simple style as the same as panic!. And all the style is the same and well such that it may be easier to be accepted.

@mina86
Copy link
Author

mina86 commented Jan 18, 2022

It may be good just call ignore!("The message") in the test case then the test case is ignored. Besides, ignore! is a clear and simple style as the same as panic!. And all the style is the same and well such that it may be easier to be accepted.

This would leave --ignored unsupported.

@yanganto
Copy link
Owner

yanganto commented Jan 18, 2022

It is good to discuss with the public under rust-lang, if you open a PR there, please kindly pin me in that PR. I still think ignore! is a clear solution, so we can discuss in that PR then. However, this issue just to remind me if there is a solution on rust-lang, I will fix this issue, so please help do not let the current issue go too far away.

@mina86
Copy link
Author

mina86 commented Jan 19, 2022

FYI, rust-lang/rfcs#3221

@yanganto
Copy link
Owner

yanganto commented May 1, 2022

This issue can be fixed after this PR is merged.
rust-lang/rust#96574

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending Issue can not be solved now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants