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

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

Merged
merged 1 commit into from
Apr 29, 2017
Merged

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

merged 1 commit into from
Apr 29, 2017

Conversation

cengiz-io
Copy link
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
Collaborator

r? @nikomatsakis

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

@Mark-Simulacrum
Copy link
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
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
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Apr 28, 2017

📌 Commit 837817c has been approved by nikomatsakis

@cengiz-io
Copy link
Contributor Author

cengiz-io commented Apr 28, 2017

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
Copy link
Contributor

bors commented Apr 29, 2017

⌛ 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
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
Copy link
Contributor

bors commented Apr 29, 2017

☀️ 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