Add sorting of map keys and test tweaks

[aglyzov]

No description provided.

[coveralls]

Coverage remained the same at 100.0% when pulling b5fd28d on aglyzov:master into f49763a on go-test:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 6aa3df7 on aglyzov:master into f49763a on go-test:master.

[daniel-nichter]

Why sort map keys before comparing them? Also, why make the test part of the pkg?

[aglyzov]

1. well, sorting is needed if you display the diffs – you always get a consistent output which is
   (a) human friendly (easy to parse with eyes, i.e. you know where to look for "it")
   (b) machine friendly (and this was my use case actually – I have hundreds of items to compare in a regression test and I use the diff program to show changes from the last text snapshot)

2. simply because without it a fork of the project is untestable (the name changes and that breaks the import)

[omeid]

Please sort the diff result, not the map, please don't make deep mutate the test data.

[aglyzov]

@omeid, the patch does exactly this: sorts the diff results before they are passed to the caller. And of course nobody mutates the outside data.

[aglyzov]

Here, let me show an example, suppose we have the diffs logged like this:

FAIL: primary/avbt_ru-list.html     : FALSE POSITIVE [map[main:true prices:[[114200 RUB]] title:Kaiser EH 6000 Lift Glas] != <nil map>]
FAIL: primary/e96_ru-list.html      : FALSE POSITIVE [map[main:true prices:[[9190 RUB] [9010 RUB]] sku:89094 title:Мониторы в Екатеринбурге] != <nil map>]
FAIL: primary/eldorado_ru-list.html : FALSE POSITIVE [map[main:true prices:[[30029 RUB]] title:Моющие пылесосы] != <nil map>]

And suppose also we have hundreds of them. So in order to have some control on what's going on we save the test output to a file and then use the diff program to check what has changed. Like this: make test-regressions | diff -u regression-tests-snapshot.txt -

But, there is a catch – due to the non-deterministic nature of hash-maps their keys come out in unpredictable order. I.e. next time it could be FALSE POSITIVE [map[title:* main:* prices:*] and the diff program will report the change where there is none.

[mibk]

I agree that the output should be deterministic. That's always better. Anyway, I wouldn't make deep_test.go a part of the package. The argument

Removing unnecessary reference to “github.com/go-test/deep” helps testing github forks.
It is safe to have the testing a part of the package because *_test.go files only get compiled when go test is invoked.

isn't solid enough. See http://blog.campoy.cat/2014/03/github-and-go-forking-pull-requests-and.html for handling GitHub forks.

Then, the 1st and the 3rd commit should be squashed.

[CpuID]

Any movement on this? +100 for sorted deterministic diffs, to allow machine parsing them to validate expectations/spot regressions.

[Anaminus]

As of Go 1.12, fmt prints maps in key-sorted order.

[daniel-nichter]

Agreed that deterministic results/output are ideal, especially for testing. Since the Go change @Anaminus mentioned, I think this feature needs to be reimplemented in a new PR. (Also, as @mibk said, deep_test.go does not need to be part of the pkg.)

Will close this PR for now, but again: I support the idea. Just need a cleaner, go1.12-aware implementation of it.