From db1ebf78a82f17f30a0636541a798a141e004770 Mon Sep 17 00:00:00 2001 From: Ian Cottrell Date: Tue, 1 Oct 2019 21:15:21 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Rebecca Stambler --- internal/lsp/diff/difftest/difftest.go | 197 +++++++++++++++++++++++-- internal/lsp/diff/myers/diff_test.go | 194 ------------------------ 2 files changed, 181 insertions(+), 210 deletions(-) diff --git a/internal/lsp/diff/difftest/difftest.go b/internal/lsp/diff/difftest/difftest.go index 338758a8b5..eaaaa41667 100644 --- a/internal/lsp/diff/difftest/difftest.go +++ b/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", - out: "gargantuan", + in: "gargantuan\n", + out: "gargantuan\n", + }, { + name: "replace_all", + in: "gord\n", + out: "gourd\n", + unified: unifiedPrefix + ` +@@ -1 +1 @@ +-gord ++gourd +`[1:], }, { name: "insert_rune", - in: "gord", - out: "gourd", + in: "gord\n", + out: "gourd\n", + unified: unifiedPrefix + ` +@@ -1 +1 @@ +-gord ++gourd +`[1:], }, { name: "delete_rune", - in: "groat", - out: "goat", + in: "groat\n", + out: "goat\n", + unified: unifiedPrefix + ` +@@ -1 +1 @@ +-groat ++goat +`[1:], }, { name: "replace_rune", - in: "loud", - out: "lord", + in: "loud\n", + out: "lord\n", + unified: unifiedPrefix + ` +@@ -1 +1 @@ +-loud ++lord +`[1:], }, { name: "insert_line", in: "one\nthree\n", out: "one\ntwo\nthree\n", - }} { - edits := compute(span.FileURI("/"+test.name), test.in, test.out) - got := diff.ApplyEdits(test.in, edits) - if got != test.out { - t.Logf("test %v had diff:%v\n", test.name, diff.ToUnified(test.name+".orig", test.name, test.in, edits)) - t.Errorf("diff %v got:\n%v\nexpected:\n%v", test.name, got, test.out) - } + unified: unifiedPrefix + ` +@@ -1,2 +1,3 @@ + one ++two + three +`[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 +} diff --git a/internal/lsp/diff/myers/diff_test.go b/internal/lsp/diff/myers/diff_test.go index 9e6d7e7842..bce0399c58 100644 --- a/internal/lsp/diff/myers/diff_test.go +++ b/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 -}