Refactor everything #53
Conversation
|
This is the coverage after refactoring the tests but before moving code around https://coveralls.io/builds/8655582 it is the same coverage as the current master. |
zimmski
force-pushed
the
43-refactor
branch
2 times, most recently
from
627132d to
435c112
Compare
|
@sergi: It would be good if you could give me some feedback on this. I will refactor the rest of the code like in the last commit here 435c112. I will basically refactor everything to be of one consistent code style, cleanup wherever I see something that could be done immediately (or add a "help wanted" issue) and basically make everything use Go idioms. |
diffmatchpatch/dmp_test.go
Outdated
| func Benchmark_DiffMainLarge(b *testing.B) { | ||
| s1 := readFile("speedtest1.txt", b) | ||
| s2 := readFile("speedtest2.txt", b) | ||
| s1 := readFile("../testdata/speedtest1.txt", b) |
There was a problem hiding this comment.
introduce constant for "../testdata", maybe even for speedtest1/2.txts
| b := "I am the very model of a modern major general,\nI've information vegetable, animal, and mineral,\nI know the kings of England, and I quote the fights historical,\nFrom Marathon to Waterloo, in order categorical.\n" | ||
| func BenchmarkDiffMain(bench *testing.B) { | ||
| s1 := "`Twas brillig, and the slithy toves\nDid gyre and gimble in the wabe:\nAll mimsy were the borogoves,\nAnd the mome raths outgrabe.\n" | ||
| s2 := "I am the very model of a modern major general,\nI've information vegetable, animal, and mineral,\nI know the kings of England, and I quote the fights historical,\nFrom Marathon to Waterloo, in order categorical.\n" |
There was a problem hiding this comment.
s1, s2 is not necessarily better than a, b
There was a problem hiding this comment.
That is true, but the whole code base uses *1&*2 more often than just a&b
| s2 := "I am the very model of a modern major general,\nI've information vegetable, animal, and mineral,\nI know the kings of England, and I quote the fights historical,\nFrom Marathon to Waterloo, in order categorical.\n" | ||
|
|
||
| // Increase the text lengths by 1024 times to ensure a timeout. | ||
| for x := 0; x < 10; x++ { |
There was a problem hiding this comment.
well, whatever this is and does i hope it does its job
| // Null case. | ||
| assert.Equal(t, 0, dmp.DiffCommonPrefix("abc", "xyz"), "'abc' and 'xyz' should not be equal") | ||
| func TestDiffCommonPrefix(t *testing.T) { | ||
| type TestCase struct { |
There was a problem hiding this comment.
theres a lot of similar TestCase types, unify and extract
There was a problem hiding this comment.
I prefer to not reuse the TestCase types. The reason is that that way I can keep them inside the test function. They are not exported and the way a test function is made stays always the same without the need to find the right TestCase type.
| dmp := New() | ||
| dmp.DiffTimeout = 200 * time.Millisecond | ||
|
|
||
| a := "`Twas brillig, and the slithy toves\nDid gyre and gimble in the wabe:\nAll mimsy were the borogoves,\nAnd the mome raths outgrabe.\n" |
| dmp.DiffTimeout = 200 * time.Millisecond | ||
|
|
||
| a := "`Twas brillig, and the slithy toves\nDid gyre and gimble in the wabe:\nAll mimsy were the borogoves,\nAnd the mome raths outgrabe.\n" | ||
| b := "I am the very model of a modern major general,\nI've information vegetable, animal, and mineral,\nI know the kings of England, and I quote the fights historical,\nFrom Marathon to Waterloo, in order categorical.\n" |
Although "diffmatchpatch" is longer than "dmp" it will make more sense after the files are split into smaller files.
- Use table-driven tests where possible - Reduce helper code, e.q., copycats of assert.Equal - Reformat most of the code - Rename the test functions to be Go conform
This PR basically refactors the test code completely, the rest is moved around and sometimes refactored.
The goals are:
It might be a good idea to look at each commit of the PR instead of the complete changes.
Fixes #43