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.

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

Introduce 'cargotest' and the check-cargotest buildstep #32348

Merged
merged 1 commit into from Mar 23, 2016

Conversation

Projects
None yet
5 participants
@brson
Copy link
Contributor

brson commented Mar 19, 2016

This is a new suite of tests that verifies that the compiler builds specific revisions of select crates from crates.io.

It does not run by default. It is intended that bors runs these tests against all PRs, and gates on them. In this way we will make it harder still to break important swaths of the ecosystem, even on nightly.

This is a very basic implementation intended for feedback. The biggest thing it probably should do but doesn't is use a lockfile for every project it builds.

r? @alexcrichton cc @rust-lang/lang @rust-lang/libs

@@ -300,6 +305,9 @@ impl<'a> Step<'a> {
Source::ToolRustbook { stage } => {
vec![self.librustc(self.compiler(stage))]
}
Source::ToolCargoTest { stage } => {
vec![self.librustc(self.compiler(stage))]

This comment has been minimized.

@alexcrichton

alexcrichton Mar 19, 2016

Member

Looks like this doesn't actually depend on the compiler libraries, so I think this can just be libstd

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 19, 2016

Looks good to me, although I do think that we'll want to check in a lock file to ensure that tests don't break to unintended consequences.

const TEST_REPOS: &'static [(&'static str, &'static str)] = &[
("https://github.com/rust-lang/cargo",
"ff02b156f094fb83e70acd965c83c9286411914e"),
("https://github.com/retep998/winapi-rs",

This comment has been minimized.

@retep998

This comment has been minimized.

@retep998

retep998 Mar 19, 2016

Member

Also it should be worth pointing out that building cargo will effectively build winapi anyway.

This comment has been minimized.

@brson

brson Mar 21, 2016

Author Contributor

Oh, good point. I'll remove winapi then. No sense doubling the work.

Edit: I assume winapi doesn't have many or any tests that we could be running. That would be a reason to leave it on the list.

This comment has been minimized.

@retep998

retep998 Mar 22, 2016

Member

Winapi does have tests, but they're almost entirely "Does this function link?" and "Is this type the right size and alignment?" so I don't mind if those tests aren't run for cargotest. Winapi being built and linked into cargo should be enough to catch the important regressions.

@aturon

This comment has been minimized.

Copy link
Member

aturon commented Mar 21, 2016

🏆 this is great to see happening!

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 21, 2016

OK, I'm going to remove the winapi crate and just land this. Cargo already has a lockfile so the test itself doesn't need explicit support.

This whole effort may simply end at testing cargo on every commit.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 22, 2016

It may also be prudent to pull in some of the gl/opengl/gfx/glutin/glium ecosystem over time as well, those also seem to be quite sizable projects which have a risk of breakage

@brson brson force-pushed the brson:cargotest branch 2 times, most recently from d4cd11b to 6493019 Mar 22, 2016

@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 22, 2016

I've replaced winapi with iron, added lockfiles and the 'cargotest' makefile target.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Mar 22, 2016

@bors: r+ 6493019

Exciting!

@brson brson force-pushed the brson:cargotest branch from 6493019 to 3a790ac Mar 22, 2016

Introduce 'cargotest' and the check-cargotest buildstep
This is a new suite of tests that verifies that the compiler
builds specific revisions of select crates from crates.io.

It does not run by default. It is intended that buildbot
runs these tests against all PRs, and gate on them.
@brson

This comment has been minimized.

Copy link
Contributor Author

brson commented Mar 22, 2016

@bors r=alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 22, 2016

📌 Commit 3a790ac has been approved by alexcrichton

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Mar 23, 2016

⌛️ Testing commit 3a790ac with merge d6af19b...

bors added a commit that referenced this pull request Mar 23, 2016

Auto merge of #32348 - brson:cargotest, r=alexcrichton
Introduce 'cargotest' and the check-cargotest buildstep

This is a new suite of tests that verifies that the compiler builds specific revisions of select crates from crates.io.

It does not run by default. It is intended that bors runs these tests against all PRs, and gates on them. In this way we will make it harder still to break important swaths of the ecosystem, even on nightly.

This is a very basic implementation intended for feedback. The biggest thing it probably should do but doesn't is use a lockfile for every project it builds.

r? @alexcrichton cc @rust-lang/lang @rust-lang/libs

@bors bors merged commit 3a790ac into rust-lang:master Mar 23, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.