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

Refactor libtest #65503

Merged
merged 8 commits into from
Oct 22, 2019
Merged

Refactor libtest #65503

merged 8 commits into from
Oct 22, 2019

Conversation

popzxc
Copy link
Contributor

@popzxc popzxc commented Oct 17, 2019

Short overview

libtest got refactored and splitted into smaller modules

Description

libtest module is already pretty big and hard to understand. Everything is mixed up: CLI, console output, test execution, etc.

This PR splits libtest into smaller logically-consistent modules, makes big functions smaller and more readable, and adds more comments, so libtest will be easier to understand and maintain.

Although there are a lot of changes, all the refactoring is "soft", meaning that no public interfaces were affected and nothing should be broken.

Thus this PR (at least should be) completely backward-compatible.

r? @wesleywiser
cc @Centril

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2019
@popzxc
Copy link
Contributor Author

popzxc commented Oct 17, 2019 •
edited
Loading

Well, seems like I've broken something =_= I will investigate it and fix tomorrow.

@popzxc
Copy link
Contributor Author

popzxc commented Oct 18, 2019

Well, I forgot to re-export public bench interface. Everything is OK now (I hope).

wesleywiser
wesleywiser approved these changes Oct 18, 2019
View reviewed changes
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

looks good and ci is green

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 18, 2019

📌 Commit a06b205 has been approved by wesleywiser

@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 Oct 18, 2019
@bors
Copy link
Contributor

bors commented Oct 20, 2019

⌛ Testing commit a06b205 with merge 2f58955296a705deee35afb6edd0edad6f489650...

@rust-highfive
Copy link
Collaborator

The job dist-various-1 of your PR failed (pretty log, raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
2019-10-20T19:42:13.8332950Z   local time: Sun Oct 20 19:42:13 UTC 2019
2019-10-20T19:42:13.8333154Z   network time: Sun, 20 Oct 2019 19:42:13 GMT
2019-10-20T19:42:13.8333335Z == end clock drift check ==
2019-10-20T19:42:15.1997551Z 
2019-10-20T19:42:15.2103341Z ##[error]Bash exited with code '1'.
2019-10-20T19:42:15.2141335Z ##[section]Starting: Upload CPU usage statistics
2019-10-20T19:42:15.2158367Z ==============================================================================
2019-10-20T19:42:15.2158464Z Task         : Bash
2019-10-20T19:42:15.2158713Z Description  : Run a Bash script on macOS, Linux, or Windows

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 @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Oct 20, 2019

💔 Test failed - checks-azure

@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 Oct 20, 2019
@popzxc
Copy link
Contributor Author

popzxc commented Oct 21, 2019 •
edited
Loading

Hm. There was cfg(unix) attr for get_concurrency which was not required for cfg(target_os = "redox").

Should be fixed now.

P.S. It would be nice to have a tool which will check any configuration combination to be valid to detect things like this without waiting a merge queue to fail.

@wesleywiser
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Oct 21, 2019

📌 Commit ae04dc8 has been approved by wesleywiser

@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 Oct 21, 2019
@Centril
Copy link
Contributor

Centril commented Oct 21, 2019

@bors rollup=never p=1

@bors
Copy link
Contributor

bors commented Oct 22, 2019

⌛ Testing commit ae04dc8 with merge 57bfb80...

bors added a commit that referenced this pull request Oct 22, 2019
@bors
Auto merge of #65503 - popzxc:refactor-libtest, r=wesleywiser
57bfb80 
Refactor libtest

## Short overview

`libtest` got refactored and splitted into smaller modules

## Description

`libtest` module is already pretty big and hard to understand. Everything is mixed up: CLI, console output, test execution, etc.

This PR splits `libtest` into smaller logically-consistent modules, makes big functions smaller and more readable, and adds more comments, so `libtest` will be easier to understand and maintain.

Although there are a lot of changes, all the refactoring is "soft", meaning that no public interfaces were affected and nothing should be broken.

Thus this PR (at least should be) completely backward-compatible.

r? @wesleywiser
cc @Centril
@bors
Copy link
Contributor

bors commented Oct 22, 2019

☀️ Test successful - checks-azure
Approved by: wesleywiser
Pushing 57bfb80 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 22, 2019
@bors bors merged commit ae04dc8 into rust-lang:master Oct 22, 2019
@bors bors mentioned this pull request Oct 22, 2019
Merged
@popzxc popzxc deleted the refactor-libtest branch October 22, 2019 15:48
@tesuji tesuji mentioned this pull request Oct 23, 2019
Merged
bors added a commit that referenced this pull request Oct 23, 2019
@bors
Auto merge of #65713 - lzutao:clippy-up, r=Manishearth
d6e4028 
Update clippy

Replaces #65690
cc  #65503
Closes #65683
@RalfJung RalfJung mentioned this pull request Oct 23, 2019
Closed
@RalfJung
Copy link
Member

RalfJung commented Oct 24, 2019 •
edited
Loading

Looks like this broke the external compiletest-rs crate (used by Clippy and Miri): Manishearth/compiletest-rs#197

Miri is fine right now by using the "stable" feature flag. EDIT: looks like some types were made public again for clippy.

@Mark-Simulacrum Mark-Simulacrum mentioned this pull request Nov 2, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wesleywiser wesleywiser wesleywiser approved these changes

Assignees

@wesleywiser wesleywiser

Labels
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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@popzxc @wesleywiser @bors @rust-highfive @Centril @RalfJung