Skip to content

use diff crate for compile-fail test diagnostics #41474#41588

Merged
bors merged 1 commit into
rust-lang:masterfrom
cengiz-io:master
Apr 29, 2017
Merged

use diff crate for compile-fail test diagnostics #41474#41588
bors merged 1 commit into
rust-lang:masterfrom
cengiz-io:master

Conversation

@cengiz-io

Copy link
Copy Markdown
Contributor

Hello!

This fixes #41474

We were using a custom implementation to dump the differences between expected and actual outputs of compile-fail tests.

I removed this internal implementation and added diff crate as a new dependency to compile-fail.

Again, huge thanks to @nikomatsakis for guiding.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@Mark-Simulacrum

Copy link
Copy Markdown
Member

Could we see a before/after comparison of a failure? Unless they're the same.

@shepmaster shepmaster added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 28, 2017
@nikomatsakis

Copy link
Copy Markdown
Contributor

The code looks good to me. @cengizio, do you have a handy "before/after" comparison to show @Mark-Simulacrum?

@Mark-Simulacrum -- for context, this only affects the diffs that print when a ui test fails. The older code was using a "hand-rolled" diff algorithm that really didn't cope well with new lines being inserted etc. This code now uses the standard "diff" algorithm, so I would expect the printouts to look much easier to read.

@nikomatsakis

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Apr 28, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 837817c has been approved by nikomatsakis

@cengiz-io

cengiz-io commented Apr 28, 2017

Copy link
Copy Markdown
Contributor Author

Hello @Mark-Simulacrum!

Thanks for keeping an eye on the issue.

Is this sample good enough for you?

#41474 (comment)

Please let me know if it's not. I'd be happy to provide full samples for each case.

@bors

bors commented Apr 29, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 837817c with merge e326e86...

bors added a commit that referenced this pull request Apr 29, 2017
use diff crate for compile-fail test diagnostics #41474

Hello!

This fixes #41474

We were using a custom implementation to dump the differences between expected and actual outputs of compile-fail tests.

I removed this internal implementation and added `diff` crate as a new dependency to `compile-fail`.

Again, huge thanks to @nikomatsakis for guiding.
@Mark-Simulacrum

Copy link
Copy Markdown
Member

Yep, that's good enough for me. Just wanted to see an overall view. Thanks!

@Mark-Simulacrum Mark-Simulacrum 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 Apr 29, 2017
@bors

bors commented Apr 29, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing e326e86 to master...

@bors bors merged commit 837817c into rust-lang:master Apr 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use diff crate while evaluating compile-fail tests

6 participants