Show only stderr diff when a ui test fails

[ritiek]

Addresses #46826.

This PR will print the normalized output if expected text is empty otherwise it will just print the diff.

Should we also show a few (actual == expected) lines above & below when displaying the diff? What about indicating line numbers as well so one can quickly check mismatch lines in .stderr file?

[ritiek]

cc @oli-obk

[kennytm]

Could we still show 3 lines of unchanged output to give the context?

[ritiek]

Do you mean we should keep diff::Result::Both()? I don't quite get it, could you please explain a bit more?

[kennytm]

something like

diff::Result::Both(t, _) if context_lines_count <= 3 => {
    println!(" {}", t);
    context_lines_count += 1;
}

Mainly to emulate the diff -u display style:

                / |    (CastTy::Ptr(_), CastTy::Int(_)) |
3 lines before {  |    (CastTy::FnPtr, CastTy::Int(_)) =>
                \ |        bcx.ptrtoint(llval, ll_t_out),
                  |  - (CastTy::Int(_), CastTy::Ptr(_)) =>
                  |  -     bcx.inttoptr(llval, ll_t_out),
                  |  + (CastTy::Int(_), CastTy::Ptr(_)) => {
                  |  +     let usize_llval = bcx.intcast(llval, bcx.ccx.isize_ty(), signed);
                  |  +     bcx.inttoptr(usize_llval, ll_t_out)
                  |  + }
                / |    (CastTy::Int(_), CastTy::Float) =>
 3 lines after {  |        cast_int_to_float(&bcx, signed, llval, ll_t_in, ll_t_out),
                \ |    (CastTy::Float, CastTy::Int(IntTy::I)) =>

[ritiek]

Thanks, I get it now. I don't think this will be as straight forward and as @durka mentioned:

Sounds like reinventing features from existing diff libraries where there might be a crate that can do it nicely!

I checked out rustfmt and it looks like it already contains some functions we'll need src/tools/rustfmt/src/bin/rustfmt-format-diff.rs.

I tested it out by copying make_diff() and other depending parts to runtest.rs and modifying compare_output() with:

fn compare_output(&self, kind: &str, actual: &str, expected: &str) -> usize {
    if actual == expected {
        return 0;
    }

    if expected.is_empty() {
        println!("normalized {}:\n{}\n", kind, actual);
    } else {
        println!("diff of {}:\n", kind);
        let results = make_diff(expected, actual, 3);
        for result in results {
            for line in result.lines {
                match line {
                    DiffLine::Expected(e) => println!("+{}", e),
                    DiffLine::Context(c) => println!(" {}", c),
                    DiffLine::Resulting(r) => println!("-{}", r),
               } 
            }    
        }    
    }

...

Here is an example generated diff:

...

---- [ui] ui/issue-10969.rs stdout ----
	diff of stderr:

    |         ^
 
 error[E0618]: expected function, found `i32`
-fun with .stderr
+  --> $DIR/issue-10969.rs:16:5
    |
 16 |     i(); //~ERROR expected function, found `i32`
    |     ^^^

The actual stderr differed from the expected stderr.

...

What do you guys think?

[nikomatsakis]

That looks nice; it'd look even better with some line numbers :)

What happens if there are multiple hunks? i.e., multiple distinct parts that changed?

[ritiek]

I've made changes to show line numbers. Here is how the diff looks with committed changes:

Details

---- [ui] ui/issue-10969.rs stdout ----
	diff of stderr:

5	   |     ^^^
6	   |
7	note: defined here
+	  --> $DIR/issue-10969.rs:11:9
8	   |
9	11 | fn func(i: i32) {
10	   |         ^

12	error[E0618]: expected function, found `i32`
13	  --> $DIR/issue-10969.rs:16:5
14	   |
+	16 |     i(); //~ERROR expected function, found `i32`
-	16 |     i(); //~ERROR expected a function, I found `i32`!!
16	   |     ^^^
17	   |
18	note: defined here

19	  --> $DIR/issue-10969.rs:15:9
-	i is `i32`, not a function just like I mentioned before
21	   |
22	15 |     let i = 0i32;
23	   |         ^


The actual stderr differed from the expected stderr.

My modified issue-10969.stderr:

Details

  1 error[E0618]: expected function, found `i32`
  2   --> $DIR/issue-10969.rs:12:5
  3    |
  4 12 |     i(); //~ERROR expected function, found `i32`
  5    |     ^^^
  6    |
  7 note: defined here
  8    |
  9 11 | fn func(i: i32) {
 10    |         ^
 11 
 12 error[E0618]: expected function, found `i32`
 13   --> $DIR/issue-10969.rs:16:5
 14    |
 15 16 |     i(); //~ERROR expected a function, I found `i32`!!
 16    |     ^^^
 17    |
 18 note: defined here
 19   --> $DIR/issue-10969.rs:15:9
 20 i is `i32`, not a function just like I mentioned before
 21    |
 22 15 |     let i = 0i32;
 23    |         ^
 24 
 25 error: aborting due to 2 previous errors

[nikomatsakis]

r? @nikomatsakis

[nikomatsakis]

This looks awesome =)

[nikomatsakis]

@bors r+

[bors]

📌 Commit 4054030 has been approved by nikomatsakis

[bors]

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