Improve tests - #33
Conversation
- add groups for future use - compare results to other implementations
- use test cases as seeds - add random seeds - compare results to other implementations - compare results to Hamming distance
- benchmarks use test cases by group - split benchmarks for others levenshtein implementations
new targets: - benchAll compares performance of all levenshtein distance implementations - cover runs the tests and opens a web browser displaying annotated source code - fuzz runs all fuzzing functions - perf compares performance using benchstat between the last commit and uncommitted code
|
Thanks a lot Paul for this! Please allow me to find some time to review the code. |
|
Dear me, my apologies @psadac , this totally dropped from my radar! Are you still available to merge the PR if I begin a review? |
No need to apologize, we don''t have a deadline, let's begin a review |
| strategy: | ||
| matrix: | ||
| go-version: [1.22.x, 1.23.x] | ||
| go-version: [1.23.x, 1.24.x] |
There was a problem hiding this comment.
Let's update these while we are at it.
| go test -bench="BenchmarkDistanceAgnivade" -benchmem -count=${COUNT} | tee bench_agnivade.out | ||
| go test -bench="BenchmarkDistanceArbovm" -benchmem -count=${COUNT} | tee bench_arbovm.out | ||
| go test -bench="BenchmarkDistanceDgryski" -benchmem -count=${COUNT} | tee bench_dgryski.out | ||
| benchstat -col=.name bench_agnivade.out bench_arbovm.out bench_dgryski.out | tee bench_all.out |
| @@ -1,3 +1,9 @@ | |||
| # FUNCS is the list of functions to run (all by default) | |||
| FUNCS?=./... | |||
There was a problem hiding this comment.
-fuzz takes a regex matched against fuzz function names, not a Go package pattern. The default should be something like Fuzz or . to match all fuzz functions.
|
|
||
| # cover runs the tests and opens a web browser displaying annotated source code | ||
| cover: test | ||
| @if [ $$? -eq 0 ]; then \ |
There was a problem hiding this comment.
This does not have any use. Since test is a prerequisite, Makefile will already halt it before running this target. We can remove it.
| // ---------------------------------- | ||
|
|
||
| // RandRunes generates a random array of runes of maxLen length. | ||
| func RandRunes(rnd *rand.Rand, maxLen int) []rune { |
There was a problem hiding this comment.
Let's unexport RandRunes, RandRunesChange, RandRune. No external package can import them anyways.
| Leven/Tibetan/dgryski-4 1.16µs ± 0% | ||
| goos: linux | ||
| goarch: amd64 | ||
| pkg: github.com/agnivade/levenshtein |
There was a problem hiding this comment.
I think this is already done in the latest README @psadac . We can remove this.
2 participants
I had already worked on the tests, fuzzing and benchmarks some months ago, but didn't push the changes.
A lot of changes were made to add tests on long strings and to avoid duplicating test cases.