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.

Sign up for GitHub

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

Jump to bottom

Test rustdoc. #44274

Merged
merged 1 commit into from
Sep 10, 2017
Merged

Test rustdoc. #44274

merged 1 commit into from
Sep 10, 2017

Conversation

Mark-Simulacrum
Copy link
Member

Also fixes the broken tests.

r? @alexcrichton

@Mark-Simulacrum
Copy link
Member Author

I suspect that this will fail until #44259 is backported to beta.

@alexcrichton
Copy link
Member

Hm so this is running tests for all of rustdoc's dependencies? Don't we want to only run tests for librustdoc?

@Mark-Simulacrum
Copy link
Member Author

Yeah, that's true. I've reworked it to only run for rustdoc now.

@alexcrichton
Copy link
Member

To land this now, can we disable doctests for rustdoc?

@Mark-Simulacrum
Copy link
Member Author

Sorry, I missed your comment! Yes, we can disable tests for now. I've passed --lib to cargo test for rustdoc, which should solve the problem here I believe. cc @rust-lang/dev-tools: Rustdoc's doc tests will not be enabled until next release cycle most likely, but unit tests will work after this lands.

@Mark-Simulacrum Mark-Simulacrum added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2017
@oli-obk
Copy link
Contributor

oli-obk commented Sep 5, 2017

#43628 adds support for disabling tests or building of tools conveniently via src/tools/toolstate.toml. We can trivially insert rustdoc into that file once the PR is merged

@Mark-Simulacrum
Copy link
Member Author

I don't think we want to disable tests or rustdoc; this PR disable the doc tests and as such should be merge able.

alexcrichton
alexcrichton reviewed Sep 5, 2017
View reviewed changes
src/bootstrap/check.rs Outdated
// Currently, Cargo can't run rustdoc's doc tests, but if PR #44259 is backported to beta,
// then this can be removed.
// cfg(stage(0)) / SNAP
cargo.arg("--lib");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh instead of passing --lib, could this instead specify:

[lib]
doctest = false

in the rustdoc Cargo.toml?

(that way this could just be cargo test)

alexcrichton
alexcrichton reviewed Sep 5, 2017
View reviewed changes
src/bootstrap/check.rs Outdated
let _time = util::timeit();

if target.contains("emscripten") {
cargo.env(format!("CARGO_TARGET_{}_RUNNER", envify(&target)),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can probably be omitted, we'll never test rustdoc for these architectures

@Mark-Simulacrum
Test rustdoc unit tests.
f87696b 
Doc tests are temporarily disabled until next release cycle, since
current beta Cargo errors on them. Upgrade should be smooth as the
relevant tests are already fixed in this commit.
@Mark-Simulacrum
Copy link
Member Author

Fixed.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Contributor

bors commented Sep 5, 2017

📌 Commit f87696b has been approved by alexcrichton

@arielb1 arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 5, 2017
@frewsxcv frewsxcv mentioned this pull request Sep 9, 2017
Closed
@bors
Copy link
Contributor

bors commented Sep 10, 2017

⌛ Testing commit f87696b with merge f982ff0...

bors added a commit that referenced this pull request Sep 10, 2017
@bors
Auto merge of #44274 - Mark-Simulacrum:rustdoc-tests, r=alexcrichton
f982ff0 
Test rustdoc.

Also fixes the broken tests.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Sep 10, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: alexcrichton
Pushing f982ff0 to master...

@bors bors merged commit f87696b into rust-lang:master Sep 10, 2017
@bors bors mentioned this pull request Sep 10, 2017
Merged
@QuietMisdreavus QuietMisdreavus mentioned this pull request Mar 16, 2018
Merged
bors added a commit that referenced this pull request Mar 17, 2018
@bors
Auto merge of #49090 - QuietMisdreavus:test-rustdoc-again, r=Mark-Sim…
8cabda4 
…ulacrum

re-enable testing librustdoc

This was originally put in in #44274, but #48105 accidentally hid it. This change puts librustdoc unit/doc tests back in the main test listing.

fixes #44237 (again)
@Mark-Simulacrum Mark-Simulacrum deleted the rustdoc-tests branch June 8, 2019 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexcrichton alexcrichton alexcrichton left review comments

Assignees

@alexcrichton alexcrichton

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Mark-Simulacrum @alexcrichton @oli-obk @bors @arielb1