Skip to content

Make rustdoc output deterministic for UI tests #76445

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Sep 9, 2020

Conversation

jyn514
Copy link
Member

@jyn514 jyn514 commented Sep 7, 2020

Closes #76442 (hopefully, since it's non-deterministic I don't have a way to test).

r? @Mark-Simulacrum
cc @GuillaumeGomez

@jyn514 jyn514 added A-testsuite Area: The testsuite used to check the correctness of rustc T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Sep 7, 2020
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 7, 2020
@GuillaumeGomez
Copy link
Member

Well, seems like the only way to do it so I guess it's fine. 👍

@Mark-Simulacrum
Copy link
Member

@bors r+ p=1 Causing spurious failures

It seems like something like -j1 would probably also work, but not sure how easy that is to thread through.

@bors
Copy link
Collaborator

bors commented Sep 7, 2020

📌 Commit dd8d0d578af62fbfc95b29ee8000c03cf746a1aa has been approved by Mark-Simulacrum

@bors bors 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 7, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 7, 2020

I'm not sure that rustdoc respects -j for doctests ...

@bors
Copy link
Collaborator

bors commented Sep 7, 2020

⌛ Testing commit dd8d0d578af62fbfc95b29ee8000c03cf746a1aa with merge 362832fdaa4344af59685f74fbf1bdb9704300c3...

@ollie27
Copy link
Member

ollie27 commented Sep 7, 2020

I'm not sure that rustdoc respects -j for doctests ...

It does respect --test-args --test-threads=1 which is what other tests use:

// compile-flags:--test --test-args --test-threads=1

Will that not work here?

@Mark-Simulacrum
Copy link
Member

@bors retry r-

Let's try the test-threads=1 solution.

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 7, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 8, 2020

I changed this to use @ollie27's suggestion.

@jyn514 jyn514 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 8, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 8, 2020

I'm not sure that this will help, but it certainly won't hurt, and I can always split up the tests in a later PR.

@jyn514
Copy link
Member Author

jyn514 commented Sep 8, 2020

This is causing further failures (#76297 (comment)) ... @Mark-Simulacrum, @ollie27 could one of you review this?

@ollie27
Copy link
Member

ollie27 commented Sep 8, 2020

@bors r=Mark-Simulacrum,ollie27

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

📌 Commit ed4dcc0 has been approved by Mark-Simulacrum,ollie27

@bors bors 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 8, 2020
@jyn514
Copy link
Member Author

jyn514 commented Sep 8, 2020

@bors p=1 Causing spurious failures

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

⌛ Testing commit ed4dcc0 with merge 178018f921b5c02381a1169fe3ebed912494609c...

@Aaron1011
Copy link
Member

Is there an issue open to track fixing the underlying non-determinism?

@jyn514
Copy link
Member Author

jyn514 commented Sep 8, 2020

Is there an issue open to track fixing the underlying non-determinism?

I'm not sure it should be considered a bug. If you run cargo test, that can also run unit tests in any order. See #75760 - I'm not sure that's an underlying bug either, just an issue with the test suite. This is a bad example because it gives more errors, not just a different order

@bors
Copy link
Collaborator

bors commented Sep 8, 2020

💥 Test timed out

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 8, 2020
@ollie27
Copy link
Member

ollie27 commented Sep 9, 2020

@bors retry

@bors bors 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 9, 2020
@bors
Copy link
Collaborator

bors commented Sep 9, 2020

⌛ Testing commit ed4dcc0 with merge b4bdc07...

@bors
Copy link
Collaborator

bors commented Sep 9, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum,ollie27
Pushing b4bdc07 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 9, 2020
@bors bors merged commit b4bdc07 into rust-lang:master Sep 9, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 9, 2020
@jyn514 jyn514 deleted the doctests branch September 9, 2020 15:57
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rustdoc output is not deterministic for failed doctests
8 participants