[WIP] split libtest types and formatting into a libtest-utils crate #57068
Conversation
|
r? @Kimundi (rust_highfive has picked a reviewer for you, use r? to override) |
|
rust-highfive
added
the
S-waiting-on-review
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
Hmmm the *Desc structs still seem to be tied to the libtest types. Currently a Desc contains a functional test but it seems like this effort would benefit from restructuring the Desc to be orthogonal to the underlying test. Otherwise it'll be hard to conveniently reuse the formatters with other test formats. |
|
|
||
| /// Test description. | ||
| #[derive(Clone, Debug, PartialEq, Eq, Hash)] | ||
| pub struct Desc { |
There was a problem hiding this comment.
Hmmm the *Desc structs still seem to be tied to the libtest types. Currently a Desc contains a functional test
So not really. Like you can see here, the Desc struct only contains the test name, whether it is ignored, whether it should panic, or is it allowed to fail.
FWIW I think we might want a better API for this than a struct, e.g., some kind of trait, but I think this is not too bad of a start, and if we put this on a crate on crates.io, we can iterate on this using normal crate.io releases, instead of tying people to particular versions of rustc.
|
Ah missed that in my skim. Sorry! Will give it a more thorough look after the holidays.
… On Dec 22, 2018, at 17:12, gnzlbg ***@***.***> wrote:
@gnzlbg commented on this pull request.
In src/libtest/libtest-utils/src/test.rs:
> @@ -0,0 +1,85 @@
+//! Tests utilities
+
+use crate::{format::Padding, ShouldPanic};
+use std::{borrow::Cow, fmt};
+
+/// Test description.
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub struct Desc {
@djrenren
Hmmm the *Desc structs still seem to be tied to the libtest types. Currently a Desc contains a functional test
So not really. Like you can see here, the Desc struct only contains the test name, whether it is ignored, whether it should panic, or is it allowed to fail.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
☔ The latest upstream changes (presumably #57079) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thanks for this @gnzlbg! I'm not entirely certain what to do about this though. I feel like this is a good direction to go in, and libtest is one of those pieces that seems like it would hugely benefit from living outside this repository and being developed externally. So, for example, let me paint a picture of where we might take this PR to the limit.
That way, long term, we'd have a dedicated project to libtest and all the rust-lang/rust repository would contain is thin shims that translate between the stable crates.io interface and the unstable rustc interface. How's that sound? |
|
That's pretty much what I had in mind. |
|
@gnzlbg and I discussed a bit on IRC and the conclusion was that @gnzlbg would open up an internal discussion to see if others have thoughts on this, and assuming others agree I think we can just delete most of the crate in this repository, move it externally, publish it, reintegrate here, and we should be good to go! |
|
I've opened the discussion here: https://internals.rust-lang.org/t/a-path-forward-towards-re-usable-libtest-functionality-custom-test-frameworks-and-a-stable-bench-macro/9139
I'm closing this because I prefer to open a PR to do just that first. |
Do not merge: I just want to get initial feedback about the general direction. I'll close this afterwards and submit a more finished PR.
cc @djrenren @alexcrichton we'll better chat about this, but this is basically a "small" step towards allowing custom test frameworks to re-use some of the
libtestfacilities.