Skip to content

Some test code cleanup #8266

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

Merged
merged 7 commits into from
Jan 12, 2022
Merged

Some test code cleanup #8266

merged 7 commits into from
Jan 12, 2022

Conversation

camsteffen
Copy link
Contributor

changelog: none

Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 10, 2022
The two dogfood tests cannot be run concurrently since they use the same
target directory.
This was more valuable when we used the latest nightly without
specifying the toolchain version.
@camsteffen

This comment has been minimized.

bors added a commit that referenced this pull request Jan 10, 2022
Some test code cleanup

changelog: none

Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
@bors

This comment has been minimized.

@bors

This comment has been minimized.

@camsteffen

This comment has been minimized.

@bors

This comment has been minimized.

bors added a commit that referenced this pull request Jan 10, 2022
Some test code cleanup

changelog: none

Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
@bors

This comment has been minimized.

@camsteffen
Copy link
Contributor Author

@bors try

@bors
Copy link
Contributor

bors commented Jan 11, 2022

⌛ Trying commit 01ef7c7 with merge ae01756...

bors added a commit that referenced this pull request Jan 11, 2022
Some test code cleanup

changelog: none

Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
@bors
Copy link
Contributor

bors commented Jan 11, 2022

☀️ Try build successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Build commit: ae01756 (ae017569db38113d127859f257c4cb7926bec729)

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM. One more question about dogfood on windows. If you think it should be enabled feel free to r=me.

@@ -3,179 +3,26 @@
//!
//! See [Eating your own dog food](https://en.wikipedia.org/wiki/Eating_your_own_dog_food) for context

// Dogfood cannot run on Windows
#![cfg(not(windows))]
Copy link
Member

Choose a reason for hiding this comment

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

The windows and mac runners are our slowest runners in CI and also the current bottleneck. This added another 1-2min to the windows runner. So I wonder if we should only run those tests on unix. An argument against that is, that contributors using windows/mac, won't see dogfood errors until they submit a PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing this.

Maybe I can find a way to get the best of both worlds using a flag or environment variable.

@camsteffen
Copy link
Contributor Author

@bors try

bors added a commit that referenced this pull request Jan 12, 2022
Some test code cleanup

changelog: none

Mainly moves /clippy_workspace_tests into /tests and combines the two dogfood tests which can't run concurrently.
@bors
Copy link
Contributor

bors commented Jan 12, 2022

⌛ Trying commit 49960fb with merge 0d4c6c1...

@camsteffen
Copy link
Contributor Author

@bors r=flip1995

@bors
Copy link
Contributor

bors commented Jan 12, 2022

📌 Commit 90bf72c has been approved by flip1995

@bors
Copy link
Contributor

bors commented Jan 12, 2022

⌛ Testing commit 90bf72c with merge 5479024...

@flip1995
Copy link
Member

That's a neat solution! Thanks!

@bors
Copy link
Contributor

bors commented Jan 12, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 5479024 to master...

@bors bors merged commit 5479024 into rust-lang:master Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants