internal/lsp: move unified diff testing to the diff interface

This now checks at the diff.TextEdit layer rather than  myers.Op

Change-Id: I706657a6c5705f0ad7109aa81f4dce174dda5f2b
Reviewed-on: https://go-review.googlesource.com/c/tools/+/198380
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/diff/difftest/difftest.go

@@ -8,44 +8,209 @@
 package difftest
 
 import (
+	"flag"
+	"fmt"
+	"io/ioutil"
+	"os"
+	"os/exec"
+	"strings"
 	"testing"
 
 	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/span"
 )
 
+const (
+	fileA         = "from"
+	fileB         = "to"
+	unifiedPrefix = "--- " + fileA + "\n+++ " + fileB + "\n"
+)
+
+var verifyDiff = flag.Bool("verify-diff", false, "Check that the unified diff output matches `diff -u`")
+
 func DiffTest(t *testing.T, compute diff.ComputeEdits) {
 	t.Helper()
-	for _, test := range []struct{ name, in, out, unified string }{{
+	for _, test := range []struct {
+		name, in, out, unified string
+		nodiff                 bool
+	}{{
 		name: "empty",
 		in:   "",
 		out:  "",
 	}, {
 		name: "no_diff",
-		in:   "gargantuan",
+		in:   "gargantuan\n",
-		out:  "gargantuan",
+		out:  "gargantuan\n",
+	}, {
+		name: "replace_all",
+		in:   "gord\n",
+		out:  "gourd\n",
+		unified: unifiedPrefix + `
+@@ -1 +1 @@
+-gord
++gourd
+`[1:],
 	}, {
 		name: "insert_rune",
-		in:   "gord",
+		in:   "gord\n",
-		out:  "gourd",
+		out:  "gourd\n",
+		unified: unifiedPrefix + `
+@@ -1 +1 @@
+-gord
++gourd
+`[1:],
 	}, {
 		name: "delete_rune",
-		in:   "groat",
+		in:   "groat\n",
-		out:  "goat",
+		out:  "goat\n",
+		unified: unifiedPrefix + `
+@@ -1 +1 @@
+-groat
++goat
+`[1:],
 	}, {
 		name: "replace_rune",
-		in:   "loud",
+		in:   "loud\n",
-		out:  "lord",
+		out:  "lord\n",
+		unified: unifiedPrefix + `
+@@ -1 +1 @@
+-loud
++lord
+`[1:],
 	}, {
 		name: "insert_line",
 		in:   "one\nthree\n",
 		out:  "one\ntwo\nthree\n",
-	}} {
+		unified: unifiedPrefix + `
-		edits := compute(span.FileURI("/"+test.name), test.in, test.out)
+@@ -1,2 +1,3 @@
-		got := diff.ApplyEdits(test.in, edits)
+ one
-		if got != test.out {
++two
-			t.Logf("test %v had diff:%v\n", test.name, diff.ToUnified(test.name+".orig", test.name, test.in, edits))
+ three
-			t.Errorf("diff %v got:\n%v\nexpected:\n%v", test.name, got, test.out)
+`[1:],
-		}
+	}, {
+		name: "replace_no_newline",
+		in:   "A",
+		out:  "B",
+		unified: unifiedPrefix + `
+@@ -1 +1 @@
+-A
+\ No newline at end of file
++B
+\ No newline at end of file
+`[1:],
+	}, {
+		name: "delete_front",
+		in:   "A\nB\nC\nA\nB\nB\nA\n",
+		out:  "C\nB\nA\nB\nA\nC\n",
+		unified: unifiedPrefix + `
+@@ -1,7 +1,6 @@
+-A
+-B
+ C
++B
+ A
+ B
+-B
+ A
++C
+`[1:],
+		nodiff: true, // diff algorithm produces different delete/insert pattern
+	},
+		{
+			name: "replace_last_line",
+			in:   "A\nB\n",
+			out:  "A\nC\n\n",
+			unified: unifiedPrefix + `
+@@ -1,2 +1,3 @@
+ A
+-B
++C
++
+`[1:],
+		},
+		{
+			name: "mulitple_replace",
+			in:   "A\nB\nC\nD\nE\nF\nG\n",
+			out:  "A\nH\nI\nJ\nE\nF\nK\n",
+			unified: unifiedPrefix + `
+@@ -1,7 +1,7 @@
+ A
+-B
+-C
+-D
++H
++I
++J
+ E
+ F
+-G
++K
+`[1:],
+		}} {
+		t.Run(test.name, func(t *testing.T) {
+			t.Helper()
+			edits := compute(span.FileURI("/"+test.name), test.in, test.out)
+			got := diff.ApplyEdits(test.in, edits)
+			unified := diff.ToUnified("from", "to", test.in, edits)
+			if got != test.out {
+				t.Errorf("got patched:\n%v\nfrom diff:\n%v\nexpected:\n%v", got, unified, test.out)
+			}
+			if unified != test.unified {
+				t.Errorf("got diff:\n%v\nexpected:\n%v", unified, test.unified)
+			}
+			if *verifyDiff && !test.nodiff {
+				diff, err := getDiffOutput(test.in, test.out)
+				if err != nil {
+					t.Fatal(err)
+				}
+				if len(diff) > 0 {
+					diff = unifiedPrefix + diff
+				}
+				if diff != test.unified {
+					t.Errorf("unified:\n%q\ndiff -u:\n%q", test.unified, diff)
+				}
+			}
+		})
 	}
 }
+
+func getDiffOutput(a, b string) (string, error) {
+	fileA, err := ioutil.TempFile("", "myers.in")
+	if err != nil {
+		return "", err
+	}
+	defer os.Remove(fileA.Name())
+	if _, err := fileA.Write([]byte(a)); err != nil {
+		return "", err
+	}
+	if err := fileA.Close(); err != nil {
+		return "", err
+	}
+	fileB, err := ioutil.TempFile("", "myers.in")
+	if err != nil {
+		return "", err
+	}
+	defer os.Remove(fileB.Name())
+	if _, err := fileB.Write([]byte(b)); err != nil {
+		return "", err
+	}
+	if err := fileB.Close(); err != nil {
+		return "", err
+	}
+	cmd := exec.Command("diff", "-u", fileA.Name(), fileB.Name())
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		if _, ok := err.(*exec.ExitError); !ok {
+			return "", fmt.Errorf("failed to run diff -u %v %v: %v\n%v", fileA.Name(), fileB.Name(), err, string(out))
+		}
+	}
+	diff := string(out)
+	if len(diff) <= 0 {
+		return diff, nil
+	}
+	bits := strings.SplitN(diff, "\n", 3)
+	if len(bits) != 3 {
+		return "", fmt.Errorf("diff output did not have file prefix:\n%s", diff)
+	}
+	return bits[2], nil
+}

internal/lsp/diff/myers/diff_test.go

@@ -5,206 +5,12 @@
 package myers_test
 
 import (
-	"flag"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"os/exec"
-	"reflect"
-	"strings"
 	"testing"
 
-	"golang.org/x/tools/internal/lsp/diff"
 	"golang.org/x/tools/internal/lsp/diff/difftest"
 	"golang.org/x/tools/internal/lsp/diff/myers"
 )
 
-const (
-	fileA         = "a/a.go"
-	fileB         = "b/b.go"
-	unifiedPrefix = "--- " + fileA + "\n+++ " + fileB + "\n"
-)
-
-var verifyDiff = flag.Bool("verify-diff", false, "Check that the unified diff output matches `diff -u`")
-
 func TestDiff(t *testing.T) {
 	difftest.DiffTest(t, myers.ComputeEdits)
 }
-
-func TestMyersDiff(t *testing.T) {
-	for _, test := range []struct {
-		a, b       string
-		lines      []*myers.Op
-		operations []*myers.Op
-		unified    string
-		nodiff     bool
-	}{
-		{
-			a:          "A\nB\nC\n",
-			b:          "A\nB\nC\n",
-			operations: []*myers.Op{},
-			unified: `
-`[1:]}, {
-			a: "A\n",
-			b: "B\n",
-			operations: []*myers.Op{
-				&myers.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&myers.Op{Kind: diff.Insert, Content: []string{"B\n"}, I1: 1, I2: 1, J1: 0},
-			},
-			unified: `
-@@ -1 +1 @@
--A
-+B
-`[1:]}, {
-			a: "A",
-			b: "B",
-			operations: []*myers.Op{
-				&myers.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&myers.Op{Kind: diff.Insert, Content: []string{"B"}, I1: 1, I2: 1, J1: 0},
-			},
-			unified: `
-@@ -1 +1 @@
--A
-\ No newline at end of file
-+B
-\ No newline at end of file
-`[1:]}, {
-			a: "A\nB\nC\nA\nB\nB\nA\n",
-			b: "C\nB\nA\nB\nA\nC\n",
-			operations: []*myers.Op{
-				&myers.Op{Kind: diff.Delete, I1: 0, I2: 1, J1: 0},
-				&myers.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 0},
-				&myers.Op{Kind: diff.Insert, Content: []string{"B\n"}, I1: 3, I2: 3, J1: 1},
-				&myers.Op{Kind: diff.Delete, I1: 5, I2: 6, J1: 4},
-				&myers.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 7, I2: 7, J1: 5},
-			},
-			unified: `
-@@ -1,7 +1,6 @@
--A
--B
- C
-+B
- A
- B
--B
- A
-+C
-`[1:],
-			nodiff: true, // diff algorithm produces different delete/insert pattern
-		},
-		{
-			a: "A\nB\n",
-			b: "A\nC\n\n",
-			operations: []*myers.Op{
-				&myers.Op{Kind: diff.Delete, I1: 1, I2: 2, J1: 1},
-				&myers.Op{Kind: diff.Insert, Content: []string{"C\n"}, I1: 2, I2: 2, J1: 1},
-				&myers.Op{Kind: diff.Insert, Content: []string{"\n"}, I1: 2, I2: 2, J1: 2},
-			},
-			unified: `
-@@ -1,2 +1,3 @@
- A
--B
-+C
-+
-`[1:],
-		},
-		{
-			a: "A\nB\nC\nD\nE\nF\nG\n",
-			b: "A\nH\nI\nJ\nE\nF\nK\n",
-			unified: `
-@@ -1,7 +1,7 @@
- A
--B
--C
--D
-+H
-+I
-+J
- E
- F
--G
-+K
-`[1:]},
-	} {
-		a := myers.SplitLines(test.a)
-		b := myers.SplitLines(test.b)
-		ops := myers.Operations(a, b)
-		if test.operations != nil {
-			if len(ops) != len(test.operations) {
-				t.Fatalf("expected %v operations, got %v", len(test.operations), len(ops))
-			}
-			for i, got := range ops {
-				want := test.operations[i]
-				if !reflect.DeepEqual(want, got) {
-					t.Errorf("expected %v, got %v", want, got)
-				}
-			}
-		}
-		applied := myers.ApplyEdits(a, ops)
-		for i, want := range applied {
-			got := b[i]
-			if got != want {
-				t.Errorf("expected %v got %v", want, got)
-			}
-		}
-		if test.unified != "" {
-			diff := myers.ToUnified(fileA, fileB, a, ops)
-			got := fmt.Sprint(diff)
-			if !strings.HasPrefix(got, unifiedPrefix) {
-				t.Errorf("expected prefix:\n%s\ngot:\n%s", unifiedPrefix, got)
-				continue
-			}
-			got = got[len(unifiedPrefix):]
-			if test.unified != got {
-				t.Errorf("expected:\n%q\ngot:\n%q", test.unified, got)
-			}
-		}
-		if *verifyDiff && test.unified != "" && !test.nodiff {
-			diff, err := getDiffOutput(test.a, test.b)
-			if err != nil {
-				t.Fatal(err)
-			}
-			if diff != test.unified {
-				t.Errorf("unified:\n%q\ndiff -u:\n%q", test.unified, diff)
-			}
-		}
-	}
-}
-
-func getDiffOutput(a, b string) (string, error) {
-	fileA, err := ioutil.TempFile("", "myers.in")
-	if err != nil {
-		return "", err
-	}
-	defer os.Remove(fileA.Name())
-	if _, err := fileA.Write([]byte(a)); err != nil {
-		return "", err
-	}
-	if err := fileA.Close(); err != nil {
-		return "", err
-	}
-	fileB, err := ioutil.TempFile("", "myers.in")
-	if err != nil {
-		return "", err
-	}
-	defer os.Remove(fileB.Name())
-	if _, err := fileB.Write([]byte(b)); err != nil {
-		return "", err
-	}
-	if err := fileB.Close(); err != nil {
-		return "", err
-	}
-	cmd := exec.Command("diff", "-u", fileA.Name(), fileB.Name())
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		if _, ok := err.(*exec.ExitError); !ok {
-			return "", fmt.Errorf("failed to run diff -u %v %v: %v\n%v", fileA.Name(), fileB.Name(), err, string(out))
-		}
-	}
-	diff := string(out)
-	bits := strings.SplitN(diff, "\n", 3)
-	if len(bits) != 3 {
-		return "", fmt.Errorf("diff output did not have file prefix:\n%s", diff)
-	}
-	return bits[2], nil
-}