First step towards rustfix compiletest mode #50084

killercup · 2018-04-19T22:10:21Z

This is the first small step towards testing auto-fixable compiler
suggestions using compiletest. Currently, it only checks if next to a
UI test there also happens to a *.rs.fixed file, and then uses rustfix
(added as external crate) on the original file, and asserts that it
produces the fixed version.

To show that this works, I've included one such test. I picked this test
case at random (and because it was simple) -- It is not relevant to the
2018 edition. Indeed, in the near future, we want to be able to restrict
rustfix to edition-lints, so this test cast might go away soon.

In case you still think this is somewhat feature-complete, here's a
quick list of things currently missing that I want to add before telling
people they can use this:

Make this an actual compiletest mode, with test [fix] … output
and everything
Assert that fixed files still compile
Assert that fixed files produce no (or a known set of) diagnostics
output
Update update-references.sh to support rustfix
Use a published version of rustfix (i.e.: publish a new version
rustfix that exposes a useful API for this)

killercup · 2018-04-19T22:10:29Z

r? @Manishearth

Manishearth · 2018-04-19T22:18:34Z

I'd rather not have this a separate mode, and instead be a feature of UI tests themselves.

rust-highfive · 2018-04-19T22:20:02Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (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.


[00:04:51] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:52] tidy error: /checkout/src/tools/compiletest/src/runtest.rs:2591: TODO is deprecated; use FIXME
[00:04:52] tidy error: /checkout/src/tools/compiletest/src/autofix.rs: incorrect license
[00:04:53] some tidy checks failed
[00:04:53] 
[00:04:53] 
[00:04:53] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:53] 
[00:04:53] 
[00:04:53] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:53] Build completed unsuccessfully in 0:01:55
[00:04:53] Build completed unsuccessfully in 0:01:55
[00:04:53] Makefile:79: recipe for target 'tidy' failed
[00:04:53] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0cbb9d72
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

Manishearth · 2018-04-19T23:51:37Z

src/tools/compiletest/Cargo.toml

@@ -13,6 +13,7 @@ regex = "0.2"
 serde = "1.0"
 serde_json = "1.0"
 serde_derive = "1.0"
+rustfix = { git = "https://github.com/rust-lang-nursery/rustfix" }


We should publish before landing

Manishearth · 2018-04-19T23:57:13Z

src/tools/compiletest/src/autofix.rs

+    let mut new_content = String::new();
+
+    // Add the lines before the section we want to replace
+    new_content.push_str(&file_content


I feel like this API should be within rustfix itself, ideally shared with the rest of the code.

This also allocates once per suggestion.

Perhaps rustfix should have an API where it applies all suggestions in order, with a callback for asking for confirmation and stuff (which we pass as an empty callback here)

The purpose of this is both to test rustfix and the suggestions, so we need to use the same code as rustfix here.

Yes, absolutely. This is just the simplest code that works.

bors · 2018-04-21T03:11:34Z

☔ The latest upstream changes (presumably #50056) made this pull request unmergeable. Please resolve the merge conflicts.

shepmaster · 2018-04-30T00:50:28Z

Ping from triage @killercup ! Will you have time to address the review feedback soon?

killercup · 2018-04-30T11:12:28Z

I'll have time to update this PR tomorrow (thank ~~god~~ bors for May Day)

killercup · 2018-05-01T22:39:20Z

@alexcrichton asked how this PR and rustfix in general was doing.

I was only able to make little progress here today. The PR is rebased and I moved the API for fixing suggestions to the rustfix crate (still unpublished, but that shouldn't be a blocker).

Next immediate step is to add compiling of fixed files and asserting previously observed warnings are gone. After that, filtering of edition-critical lints.

rust-highfive · 2018-05-01T22:43:01Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (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.


[00:05:15] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:16] tidy error: /checkout/src/tools/compiletest/src/runtest.rs:2609: TODO is deprecated; use FIXME
[00:05:16] tidy error: /checkout/src/tools/compiletest/src/runtest.rs:2610: line longer than 100 chars
[00:05:16] tidy error: /checkout/src/tools/compiletest/src/runtest.rs:2623: TODO is deprecated; use FIXME
[00:05:17] some tidy checks failed
[00:05:17] 
[00:05:17] 
[00:05:17] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:17] 
[00:05:17] 
[00:05:17] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:17] Build completed unsuccessfully in 0:02:06
[00:05:17] Build completed unsuccessfully in 0:02:06
[00:05:17] make: *** [tidy] Error 1
[00:05:17] Makefile:79: recipe for target 'tidy' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0a9e736c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

rust-highfive · 2018-05-01T22:53:08Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (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.


[00:04:45] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:04:46] tidy error: /checkout/src/tools/compiletest/src/runtest.rs:2610: line longer than 100 chars
[00:04:47] some tidy checks failed
[00:04:47] 
[00:04:47] 
[00:04:47] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:47] 
[00:04:47] 
[00:04:47] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:47] Build completed unsuccessfully in 0:01:52
[00:04:47] Build completed unsuccessfully in 0:01:52
[00:04:47] Makefile:79: recipe for target 'tidy' failed
[00:04:47] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:3df2d08c
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

alexcrichton · 2018-05-01T23:37:43Z

Awesome thanks for the cc @killercup!

I'm curious, though, would it perhaps be best to put tests into the rustfix crate itself? I'd imagine that'd allow for much faster iteration on rustfix itself. Or is the purpose here to test suggestions as they are added to rustc, using what's likely to be a relatively stable base in rustfix?

I can try to clean up some of the todo items here tomorrow, I've had some experience in the past adding new rustfix modes and whatnot.

killercup · 2018-05-01T23:45:13Z

Thanks! Ping me if you have any questions. The latter: rustfix already has integration tests similar to compiletest (incl. simple clippy support)—this is to make sure we can trust (and don’t break) the suggestions rustc produces. Alex Crichton <notifications@github.com> schrieb am Mi. 2. Mai 2018 um 01:38:

…

Awesome thanks for the cc @killercup <https://github.com/killercup>! I'm curious, though, would it perhaps be best to put tests into the rustfix crate itself? I'd imagine that'd allow for much faster iteration on rustfix itself. Or is the purpose here to test suggestions as they are added to rustc, using what's likely to be a relatively stable base in rustfix? I can try to clean up some of the todo items here tomorrow, I've had some experience in the past adding new rustfix modes and whatnot. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#50084 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABOX14NSJthUVOgO9ARF0Nuzr6D_e92ks5tuPHdgaJpZM4Tck8n> .

alexcrichton · 2018-05-02T16:03:55Z

Ok I've pushed up a commit that makes a dedicated suite for rustfix tests and populated with a set of working tests from src/test/ui/suggestions.

I think the last step here is to publish rustfix to crates.io and get it pulled in from there.

@killercup would it be possible to split out the binary/library to avoid having compiletest depend on dependencies like clap in this repo?

killercup · 2018-05-03T07:44:38Z

Thanks so much, @alexcrichton!

@Manishearth said in #50084 (comment) he'd rather see this is part of the UI tests, probably to speed the tests up and to see the regular diagnostics at the same time.

alexcrichton · 2018-05-03T15:19:19Z

@Manishearth can you elaborate on the idea for a UI mode? The implementation here with the necessary bells and whistles seems like it's pretty different from UI and somewhat makes sense as a separate mode?

Manishearth · 2018-05-03T15:24:30Z

In general I see the process of testing such things to be the same as the process for testing things that need UI tests; i.e. if you're writing a fixture test you're probably also going to write a UI test.

This also makes it easy to "upgrade" UI tests to fixture tests by adding a .fixed file, which hopefully will improve rustc suggestions overall.

killercup · 2018-05-03T15:38:19Z

I think the "upgrade UI tests" workflow is pretty powerful: Ideally, we'd be able to make one PR soon that adds .fixed files for all the UI tests that already compile after running rustfix, and then open a "help wanted" issue with a list of UI tests whose suggestions are broken. With some good instructions this could get people involved just like the "new diagnostics" effort a few years ago.

That said, it's not important right now: We only care about a handful of lints right now.

alexcrichton · 2018-05-03T18:44:27Z

Ok sounds good to me! I've folded this back into the UI test suite with the following implementation:

The presence of // run-rustfix will execute rustfix and ensure the compiled code has no errors
Rustfix assertions are located at name.fixed
If name.fixed exists and // run-rustfix isn't specified, an error is generated

alexcrichton · 2018-05-03T18:44:32Z

r? @Manishearth

killercup · 2018-05-03T18:53:16Z

Thanks for working on this, @alexcrichton!

If name.fixed exists and // run-rustfix isn't specified, an error is generated

Why? Is this to trigger the "ensure the compiled code has no errors" thing? Would it make sense to add a .fixed.stderr file, so to run UI tests against the fixed file too?

Also: Can we rename .rs.fixed to .fixed.rs to get syntax highlighting? (I only just noticed this and I know it's my fault.)

alexcrichton · 2018-05-04T14:26:04Z

@bors: r=Manishearth

bors · 2018-05-04T14:26:04Z

📌 Commit e539c2b has been approved by Manishearth

bors · 2018-05-04T21:56:19Z

🔒 Merge conflict

This is the first small step towards testing auto-fixable compiler suggestions using compiletest. Currently, it only checks if next to a UI test there also happens to a `*.rs.fixed` file, and then uses rustfix (added as external crate) on the original file, and asserts that it produces the fixed version. To show that this works, I've included one such test. I picked this test case at random (and because it was simple) -- It is not relevant to the 2018 edition. Indeed, in the near future, we want to be able to restrict rustfix to edition-lints, so this test cast might go away soon. In case you still think this is somewhat feature-complete, here's a quick list of things currently missing that I want to add before telling people they can use this: - [ ] Make this an actual compiletest mode, with `test [fix] …` output and everything - [ ] Assert that fixed files still compile - [ ] Assert that fixed files produce no (or a known set of) diagnostics output - [ ] Update `update-references.sh` to support rustfix - [ ] Use a published version of rustfix (i.e.: publish a new version rustfix that exposes a useful API for this)

Uses branch from <rust-lang/rustfix#63> until we publish a new release.

This commit adds a dedicated mode to compiletest for running rustfix tests, adding a new `src/test/rustfix` directory which will execute all tests as a "rustfix" test, namely requiring that a `*.fixed` is next to the main file which is the result of the rustfix project's application of fixes. The `rustfix` crate is pulled in to actually perform the fixing, and the rustfix compiletest mode will assert a few properties about the fixing: * The expected fixed output must be the same as rustc's output suggestions applied to the original code. * The fixed code must compile successfully * The fixed code must have no further diagnostics emitted about it

alexcrichton · 2018-05-04T22:01:50Z

@bors: r=Manishearth

bors · 2018-05-04T22:01:51Z

📌 Commit 6f2d023 has been approved by Manishearth

bors · 2018-05-04T23:15:07Z

⌛ Testing commit 6f2d023 with merge 1d16826...

First step towards rustfix compiletest mode This is the first small step towards testing auto-fixable compiler suggestions using compiletest. Currently, it only checks if next to a UI test there also happens to a `*.rs.fixed` file, and then uses rustfix (added as external crate) on the original file, and asserts that it produces the fixed version. To show that this works, I've included one such test. I picked this test case at random (and because it was simple) -- It is not relevant to the 2018 edition. Indeed, in the near future, we want to be able to restrict rustfix to edition-lints, so this test cast might go away soon. In case you still think this is somewhat feature-complete, here's a quick list of things currently missing that I want to add before telling people they can use this: - [x] Make this an actual compiletest mode, with `test [fix] …` output and everything - [x] Assert that fixed files still compile - [x] Assert that fixed files produce no (or a known set of) diagnostics output - [x] Update `update-references.sh` to support rustfix - [x] Use a published version of rustfix (i.e.: publish a new version rustfix that exposes a useful API for this)

bors · 2018-05-05T01:27:33Z

☀️ Test successful - status-appveyor, status-travis
Approved by: Manishearth
Pushing 1d16826 to master...

…Simulacrum Remove vestigial rustfix tests. The directory `src/test/rustfix` is not actually tested. It looks like a mistake was made when rustfix tests were originally introduced in rust-lang#50084. In commit 6f2d023 they were moved to `src/test/ui`, but the tests in the original directory weren't deleted.

kennytm added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 19, 2018

Manishearth requested changes Apr 19, 2018

View reviewed changes

pietroalbini assigned Manishearth Apr 20, 2018

This was referenced May 1, 2018

Provide rustfix to upgrade code to Rust 2018 automatically #49247

Closed

Apply suggestion rust-lang/rustfix#63

Merged

killercup force-pushed the compiletest-rustfix branch from 6c58c6a to 57aa8d8 Compare May 1, 2018 22:34

killercup force-pushed the compiletest-rustfix branch from 9bee46b to 54e384f Compare May 1, 2018 22:54

alexcrichton force-pushed the compiletest-rustfix branch 2 times, most recently from 9c4ffbf to 4ea828b Compare May 2, 2018 16:02

killercup mentioned this pull request May 3, 2018

Split rustfix into two crates rust-lang/rustfix#65

Merged

alexcrichton changed the title ~~WIP: First step towards rustfix compiletest mode~~ First step towards rustfix compiletest mode May 3, 2018

alexcrichton force-pushed the compiletest-rustfix branch from 60d78b4 to e539c2b Compare May 4, 2018 14:25

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2018

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 May 4, 2018

killercup and others added 7 commits May 4, 2018 15:01

Use rustfix' suggestion fixing API

c02aedf

Uses branch from <rust-lang/rustfix#63> until we publish a new release.

Skip NLL compiletest in rustfix mode for now

9680f3b

tidy

b264504

Use published rustfix 0.2 version

a563027

Fold rustfix tests back into the UI test suite

6f2d023

alexcrichton force-pushed the compiletest-rustfix branch from e539c2b to 6f2d023 Compare May 4, 2018 22:01

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 4, 2018

bors merged commit 6f2d023 into rust-lang:master May 5, 2018

This was referenced May 5, 2018

Misc tweaks #50276

Merged

compiletest: Run revisions as independent tests. #50400

Merged

ehuss mentioned this pull request May 9, 2018

Rollup of 11 pull requests #50546

Merged

pnkfelix mentioned this pull request Sep 4, 2018

Need compare-mode=2018 (or similar) to run test suite under multiple editions #52979

Open

ehuss mentioned this pull request Aug 27, 2021

Remove vestigial rustfix tests. #88387

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

First step towards rustfix compiletest mode #50084

First step towards rustfix compiletest mode #50084

killercup commented Apr 19, 2018 •

edited

Loading

killercup commented Apr 19, 2018

Manishearth commented Apr 19, 2018

rust-highfive commented Apr 19, 2018

Manishearth Apr 19, 2018

Manishearth Apr 19, 2018

killercup Apr 20, 2018

bors commented Apr 21, 2018

shepmaster commented Apr 30, 2018

killercup commented Apr 30, 2018

killercup commented May 1, 2018

rust-highfive commented May 1, 2018

rust-highfive commented May 1, 2018

alexcrichton commented May 1, 2018

killercup commented May 1, 2018 via email

alexcrichton commented May 2, 2018

killercup commented May 3, 2018

alexcrichton commented May 3, 2018

Manishearth commented May 3, 2018

killercup commented May 3, 2018

alexcrichton commented May 3, 2018

alexcrichton commented May 3, 2018

killercup commented May 3, 2018

alexcrichton commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

alexcrichton commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

bors commented May 5, 2018

First step towards rustfix compiletest mode #50084

First step towards rustfix compiletest mode #50084

Conversation

killercup commented Apr 19, 2018 • edited Loading

killercup commented Apr 19, 2018

Manishearth commented Apr 19, 2018

rust-highfive commented Apr 19, 2018

Manishearth Apr 19, 2018

Choose a reason for hiding this comment

Manishearth Apr 19, 2018

Choose a reason for hiding this comment

killercup Apr 20, 2018

Choose a reason for hiding this comment

bors commented Apr 21, 2018

shepmaster commented Apr 30, 2018

killercup commented Apr 30, 2018

killercup commented May 1, 2018

rust-highfive commented May 1, 2018

rust-highfive commented May 1, 2018

alexcrichton commented May 1, 2018

killercup commented May 1, 2018 via email

alexcrichton commented May 2, 2018

killercup commented May 3, 2018

alexcrichton commented May 3, 2018

Manishearth commented May 3, 2018

killercup commented May 3, 2018

alexcrichton commented May 3, 2018

alexcrichton commented May 3, 2018

killercup commented May 3, 2018

alexcrichton commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

alexcrichton commented May 4, 2018

bors commented May 4, 2018

bors commented May 4, 2018

bors commented May 5, 2018

killercup commented Apr 19, 2018 •

edited

Loading