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

fix for recent nightlies' TestOpts change #30

Merged
merged 7 commits into from
Feb 21, 2022
Merged

Conversation

cormacrelf
Copy link
Contributor

@cormacrelf cormacrelf commented Mar 18, 2021

See rust-lang/rust#81356. The filter: Option<String> field was changed to filters: Vec<String>.

@cormacrelf
Copy link
Contributor Author

cormacrelf commented Mar 18, 2021

Clippy failure wise... yeah, it will fail on stable. It's using unstable nightly APIs that are different between nightly and stable, it literally cannot pass clippy stable. The other two problems are fixed in the other PR.

Edit: not now that I conditionally included the change using version_check.

... about the using the specialization feature. datatest can't only use
min_specialization, apparently (I tried this).
https://doc.rust-lang.org/stable/test/struct.TestOpts.html

vs

https://doc.rust-lang.org/nightly/test/struct.TestOpts.html

The `filter: Option<String>` field was changed to `filters:
Vec<String>`. So this revision uses version_check to determine
whether libtest has that update. Makes it compile on both
stable (1.50) and nightly (1.52) at the time of writing.
It disabled too much stuff through the use
of a non-existent cfg(feature="stable"). That now exists as
feature="rustc_is_stable", same with rustc_is_nightly.

Then, allows test_case_registration to become a default feature.
Essentially guards its use in cfg() with &= rustc_is_nightly, so stable
users do not have to --no-default-features.

Each test suite has instructions for running it at the top now.
@cormacrelf
Copy link
Contributor Author

Ok, so a few updates:

  • I made it compile on stable as well by using version_check.
  • I think datatest is, by about rust 1.52, maybe going to stop working on stable. See build.rs for a note on that.
  • The last commit making test_case_registration the default is optional, I'm not sure if it is a breaking change or not. Probably not? The whole test suite passes with or without it. Maybe you didn't use it by default previously because that API might be a bit less stable than the ctor method. I've missed a bit but feel free to merge without it.

@mverleg
Copy link

mverleg commented Mar 28, 2021

Used this PR in my project and it solved the compile issue and runs the data-dependent tests successfully.

@cormacrelf
Copy link
Contributor Author

I'll be sure to keep my main branch clean so that keeps working until this is merged.

@eranrund
Copy link

Hi! Any estimation on when this could get merged?
Thanks!

@mverleg
Copy link

mverleg commented Feb 27, 2022

Thanks for merging

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.

4 participants