-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
relax rustdoc tests #4451
relax rustdoc tests #4451
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
tests/rustdoc.rs
Outdated
.with_stderr_contains("[RUNNING] `rustdoc --crate-name foo src[/]lib.rs") | ||
.with_stderr_contains(&format!("-o {}[/]target[/]doc", p.root())) | ||
.with_stderr_contains(&format("-L dependency={dir}[/]target[/]debug[/]deps`", p.root())) | ||
.with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this test case doesn't need to change, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I just changed them all for consistency. I can un-do that if you'd prefer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'd personally prefer to keep the exhaustive output checks, and if we use --cfg
, a non-deprecated flag, I think that'll be ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's see what bors says :)
tests/rustdoc.rs
Outdated
.with_stderr_contains(&format!("-o {}[/]target[/]doc", p.root().display())) | ||
.with_stderr_contains("--no-defaults") | ||
.with_stderr_contains(&format!("-L dependency={}[/]target[/]debug[/]deps`", p.root().display())) | ||
.with_stderr_contains("[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this instead just change to testing something like passing the --cfg
flag? (along with tests below)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
8eee961
to
11bd376
Compare
I am having trouble getting some tests to run locally at the moment, so I'd like to see this pass, try rust-lang/rust#44138 with it, and then only r+ it after both things have run to ensure that this does actually fix the upstream issue. |
Hm what's the output of I think these'll need to change the test assertions as well which look to still be using |
Ah it's not that, it's the stdlib tests. And yeah, you're right, my bad. Ugh. |
In rust-lang/rust#44138 we are adding deprecations, and so it breaks these tests.
11bd376
to
b1e8298
Compare
@bors: r+ |
📌 Commit b1e8298 has been approved by |
relax rustdoc tests This was asserting on the output directly, rather than just what it contains. In rust-lang/rust#44138 we are adding deprecations, and so it breaks these tests.
☀️ Test successful - status-appveyor, status-travis |
Part of rust-lang#44136 Upgrades cargo due to rust-lang/cargo#4451
This was asserting on the output directly, rather than just what it contains.
In rust-lang/rust#44138 we are adding deprecations, and so it
breaks these tests.