extend compiletest to support revisions, incremental tests #32007

nikomatsakis · 2016-03-02T10:15:26Z

This PR extends compiletest to support test revisions and with a preliminary incremental testing harness. run-pass, compile-fail, and run-fail tests may be tagged with

// revisions: a b c d

This will cause the test to be re-run four times with --cfg {a,b,c,d} in turn. This means you can write very closely related things using cfg. You can also configure the headers/expected-errors by writing //[foo] header: value or //[foo]~ ERROR bar, where foo is the name of your revision. See the changes to coherence-cow.rs as a proof of concept.

The main point of this work is to support the incremental testing harness. This PR contains an initial, unused version. The code that uses it will land later. The incremental testing harness compiles each revision in turn, and requires that the revisions have particular names (e.g., rpass2, cfail3), which tell it whether a particular revision is expected to compile or not.

Two questions:

Is there compiletest documentation anywhere I can update?
Should I hold off on landing the incremental testing harness until I have the code to exercise it? (That will come in a separate PR, still fixing a few details)

r? @alexcrichton
cc @rust-lang/compiler <-- new testing capabilities

a test file may specify `// revisions: foo bar baz` headers and expected errors may be made specific to a revision by writing `//[foo] header` or `//[foo]~ ERROR`

eddyb · 2016-03-02T10:25:33Z

This is great! LLVM's FileCheck has a similar feature, where you can include several test commands in a single file and a prefix for each one such that you check each of their outputs independently.

I wonder if we could extend this to work for MIR or even LLVM IR tests, to avoid depending on FileCheck.

nikomatsakis · 2016-03-02T11:19:32Z

PS, it'd be nice if we could write automated tests for compiletest itself, but I don't know of any way to do that right now... but I have manually tested this ;)

eddyb · 2016-03-02T11:21:28Z

@nikomatsakis Would a marker for "this test must fail (according to compiletest)" suffice?

nikomatsakis · 2016-03-02T11:23:26Z

@eddyb I could see two things:

add some "metatests" into the makefile that are expected to fail
or add a header switch to "invert" expectations

The latter requires us still to trust compiletest, but maybe a bit less (and of course the former requires trusting makefiles, which is probably scarier).

nikomatsakis · 2016-03-02T11:26:57Z

yeah, thinking about I like the idea of a // should-fail header. I think the meta runner of compiletest could detect and handle such a header with relative ease. I'll investigate.

apasel422 · 2016-03-02T12:13:01Z

@nikomatsakis There's some compiletest documentation in COMPILER_TESTS.md.

alexcrichton · 2016-03-02T18:47:41Z

src/compiletest/runtest.rs

@@ -1313,6 +1392,27 @@ fn ensure_dir(path: &Path) {
    fs::create_dir_all(path).unwrap();
 }

+fn remove_dir_and_contents(path: &Path) -> io::Result<()> {


Shouldn't this just be fs::remove_dir_all?

alexcrichton · 2016-03-02T18:52:15Z

Looks good to me, nice idea @nikomatsakis!

Implementation looks all good to me, although I didn't really audit the new incremental tests that closely. I also basically just skimmed the changes as "yep, looks like a refactor". Feel free to r=me whenever this is ready.

and write a really basic "meta test" of the compilertest framework

nikomatsakis · 2016-03-02T19:27:49Z

Decided I might as well land the incremental runner separately. I may well need to tweak it some more, so it seems clearer to put it in the same PR as the code which uses it.

nikomatsakis · 2016-03-02T19:27:58Z

@bors r=alexcrichton

bors · 2016-03-02T19:27:58Z

📌 Commit fff9c2b has been approved by alexcrichton

bors · 2016-03-03T07:27:12Z

⌛ Testing commit fff9c2b with merge 356759a...

bors · 2016-03-03T08:35:51Z

💔 Test failed - auto-linux-64-opt

nikomatsakis · 2016-03-03T09:18:28Z

@bors r=alexcrichton

bors · 2016-03-03T09:18:28Z

📌 Commit 9601e6f has been approved by alexcrichton

bors · 2016-03-03T12:07:14Z

⌛ Testing commit 9601e6f with merge 71669bb...

bors · 2016-03-03T13:17:09Z

💔 Test failed - auto-linux-64-opt

should-fail annotation

nikomatsakis · 2016-03-03T16:28:17Z

@bors r=alexcrichton

bors · 2016-03-03T16:28:18Z

📌 Commit fc4d0ec has been approved by alexcrichton

nikomatsakis · 2016-03-03T16:29:02Z

ok, this should work now. I made the executive call that should-fail will be ignored for pretty printing tests, since usually they are checking something different than the thing which should-fail was intending to test. Another option would have to been to add // ignore-pretty to meta tests, but this seemed to be more likely to be what we want. We can add a should-fail-pretty header if we ever want that.

bors · 2016-03-03T17:41:14Z

⌛ Testing commit fc4d0ec with merge 493d999...

@alexcrichton

…ichton This PR extends compiletest to support **test revisions** and with a preliminary **incremental testing harness**. run-pass, compile-fail, and run-fail tests may be tagged with ``` // revisions: a b c d ``` This will cause the test to be re-run four times with `--cfg {a,b,c,d}` in turn. This means you can write very closely related things using `cfg`. You can also configure the headers/expected-errors by writing `//[foo] header: value` or `//[foo]~ ERROR bar`, where `foo` is the name of your revision. See the changes to `coherence-cow.rs` as a proof of concept. The main point of this work is to support the incremental testing harness. This PR contains an initial, unused version. The code that uses it will land later. The incremental testing harness compiles each revision in turn, and requires that the revisions have particular names (e.g., `rpass2`, `cfail3`), which tell it whether a particular revision is expected to compile or not. Two questions: - Is there compiletest documentation anywhere I can update? - Should I hold off on landing the incremental testing harness until I have the code to exercise it? (That will come in a separate PR, still fixing a few details) r? @alexcrichton cc @rust-lang/compiler <-- new testing capabilities

bors · 2016-03-03T19:52:10Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-mac-ios-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt

@michaelwoerister

Save/load incremental compilation dep graph Contains the code to serialize/deserialize the dep graph to disk between executions. We also hash the item contents and compare to the new hashes. Also includes a unit test harness. There are definitely some known limitations, such as #32014 and #32015, but I am leaving those for follow-up work. Note that this PR builds on #32007, so the overlapping commits can be excluded from review. r? @michaelwoerister

@michaelwoerister

Save/load incremental compilation dep graph Contains the code to serialize/deserialize the dep graph to disk between executions. We also hash the item contents and compare to the new hashes. Also includes a unit test harness. There are definitely some known limitations, such as #32014 and #32015, but I am leaving those for follow-up work. Note that this PR builds on #32007, so the overlapping commits can be excluded from review. r? @michaelwoerister

nikomatsakis added 5 commits March 1, 2016 10:43

require that header lines begin with //

24f0851

refactor header parsing so it can work "in-place"

c761257

change compile_flags to eagerly split into a vector on whitespace

d78b4a9

introduce the notion of revisions, currently unused

bb2a425

a test file may specify `// revisions: foo bar baz` headers and expected errors may be made specific to a revision by writing `//[foo] header` or `//[foo]~ ERROR`

refactor and generalize revisions

c5bdc35

rust-highfive assigned alexcrichton Mar 2, 2016

nikomatsakis added 2 commits March 2, 2016 05:16

convert coherence-cow to use revisions

4cffa9b

thread revision identifier around in test output

3fb80e3

nikomatsakis force-pushed the compiletest-incremental branch from 5189446 to dee6853 Compare March 2, 2016 10:16

alexcrichton reviewed Mar 2, 2016
View reviewed changes

nikomatsakis added 4 commits March 2, 2016 14:26

Update COMPILER_TESTS.md documentation to discuss revisions

1d76ccd

make it possible for a test to declare should-panic

0d6b4e0

and write a really basic "meta test" of the compilertest framework

rename should-panic to should-fail, per acrichto's suggestion

ad3ed80

add a run-fail meta test

fff9c2b

nikomatsakis force-pushed the compiletest-incremental branch from f36f73d to fff9c2b Compare March 2, 2016 19:27

nikomatsakis mentioned this pull request Mar 2, 2016

Save/load incremental compilation dep graph #32016

Merged

Do not assert revisions list is empty for pretty printing tests

9601e6f

nikomatsakis added 2 commits March 3, 2016 11:17

make pretty printer tests understand revisions, and make them ignore the

aa19b41

should-fail annotation

mention caveat about should-fail in docs

fc4d0ec

bors merged commit fc4d0ec into rust-lang:master Mar 3, 2016

llogiq mentioned this pull request Mar 6, 2016

TWIR 121's CotW + PRs rust-lang/this-week-in-rust#176

Closed

nikomatsakis mentioned this pull request Mar 7, 2016

Implement RFC 1210: impl specialization #30652

Merged

nikomatsakis deleted the compiletest-incremental branch March 30, 2016 16:12

Zalathar mentioned this pull request Feb 21, 2024

Consistently refer to a test's revision instead of cfg #121373

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

extend compiletest to support revisions, incremental tests #32007

extend compiletest to support revisions, incremental tests #32007

nikomatsakis commented Mar 2, 2016

eddyb commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

eddyb commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

apasel422 commented Mar 2, 2016

alexcrichton Mar 2, 2016

alexcrichton commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

bors commented Mar 2, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

extend compiletest to support revisions, incremental tests #32007

extend compiletest to support revisions, incremental tests #32007

Conversation

nikomatsakis commented Mar 2, 2016

eddyb commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

eddyb commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

apasel422 commented Mar 2, 2016

alexcrichton Mar 2, 2016

Choose a reason for hiding this comment

alexcrichton commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

nikomatsakis commented Mar 2, 2016

bors commented Mar 2, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

nikomatsakis commented Mar 3, 2016

bors commented Mar 3, 2016

bors commented Mar 3, 2016