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

Don't use tester on rustc feature #203

Closed

Conversation

JohnTitor
Copy link
Contributor

@JohnTitor JohnTitor commented Nov 16, 2019

Currently, tester crate is used in stable, beta, and nightly as well.
But on nightly, we should use Rust's builtin test crate.
#200 made this breakage.
Also, this contains the fix for the latest rustc breakage.

@JohnTitor
Copy link
Contributor Author

Uhmm, at glance, this will cause multiple matching crates error.

@Munksgaard
Copy link
Collaborator

Currently, tester crate is used in stable, beta, and nightly as well.
But on nightly, we should use Rust's builtin test crate.
#200 made this breakage.
Also, this contains the fix for the latest rustc breakage as comments (it's not released yet).

I'm not sure I agree that we should use test by default on nightly, as it requires the user to install the rustc-dev component to work. What do you think @Manishearth, @laumann?

@JohnTitor
Copy link
Contributor Author

Ah, I picked the wrong word. The use of builtin crate is limited by rustc feature so we can also use this crate on nightly without rustc feature and rustc-dev, I think.

@JohnTitor
Copy link
Contributor Author

At least, the current master maybe causes mismatched types error because tester is no longer an optional dependency. Also, we cannot add unstable options that tester crate doesn't have.

@JohnTitor JohnTitor changed the title Don't use tester on nightly Don't use tester on rustc feature Nov 16, 2019
@Munksgaard
Copy link
Collaborator

Ah, I picked the wrong word. The use of builtin crate is limited by rustc feature so we can also use this crate on nightly without rustc feature and rustc-dev, I think.

Yes, at least that was the goal of the recent changes.

At least, the current master maybe causes mismatched types error because tester is no longer an optional dependency. Also, we cannot add unstable options that tester crate doesn't have.

That is true. It would be nice to be able to choose between tester and test, although I think tester should be the default, since it's more generally applicably and doesn't require any additional work from the user.

@messense
Copy link
Contributor

So I've just opened messense/rustc-test#2 to update tester crate to latest rust-lang/rust master

@JohnTitor
Copy link
Contributor Author

@messense it's great! I'll submit another PR once new version of tester crate is released.

@messense
Copy link
Contributor

@JohnTitor Just released tester 0.6.0 to crates.io.

@JohnTitor
Copy link
Contributor Author

Then, I'll open a PR to follow tester 0.6.0 soonish.

@JohnTitor JohnTitor closed this Nov 17, 2019
@JohnTitor JohnTitor deleted the add-force-run-in-process branch November 17, 2019 06:33
@flip1995
Copy link

Uh, I think we want this change anyway. Now that tester is not optional anymore, clippy fails to compile, even with adding features = [ "rustc" ], with:

error[E0308]: mismatched types
   --> tests/compile-test.rs:151:42
    |
151 |     let res = run_ui_toml_tests(&config, tests);
    |                                          ^^^^^ expected struct `test::TestDescAndFn`, found struct `test::types::TestDescAndFn`
    |
    = note: expected type `std::vec::Vec<test::TestDescAndFn>`
               found type `std::vec::Vec<test::types::TestDescAndFn>`
note: Perhaps two different versions of crate `test` are being used?
   --> tests/compile-test.rs:151:42
    |
151 |     let res = run_ui_toml_tests(&config, tests);
    |                                          ^^^^^

This can be fixed by adding tester = "0.6" as a dev-dependencies to Clippy. But then another test fails in our test suite, because it has a #[cfg(test)] mod test { ... } in a ui-test. -> multiple matching crates for test.


I will open a PR and paste a link to Travis here, once it failed...

@Manishearth
Copy link
Owner

That's because tester does the ridiculous thing of actually calling the library crate test, leading to this problem. Crates.io shouldn't actually allow it, but it does (I filed rust-lang/crates.io#1904).

I've filed messense/rustc-test#3 and opened a PR to fix it.

@Manishearth
Copy link
Owner

(The fact that tester does this has been annoying in the past because it's very confusing to understand compiletest code)

@flip1995
Copy link

That's because tester does the ridiculous thing of actually calling the library crate test

Yes that too. But with feature rustc enabled, tester shouldn't even be used IMO.

@Manishearth
Copy link
Owner

Yep. I'd like to move off of rustc's test anyway, though.

@messense
Copy link
Contributor

@flip1995 @Manishearth tester 0.7.0 has been released, it's now just called tester.

@Manishearth
Copy link
Owner

Thanks!

@messense
Copy link
Contributor

Opened #208 to update to tester 0.7.0.

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.

5 participants