From a951dc7dd6878760f8813d19bcb21f73c0109532 Mon Sep 17 00:00:00 2001
From: Jared Lumpe <mjlumpe@gmail.com>
Date: Sat, 4 Jul 2020 21:26:07 -0600
Subject: [PATCH 1/4] Submodule for testing utils

---
 Project.toml        |   1 +
 docs/src/dev.md     |  15 ++++++
 src/Neighborhood.jl |   1 +
 src/Testing.jl      | 111 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 128 insertions(+)
 create mode 100644 src/Testing.jl

diff --git a/Project.toml b/Project.toml
index 1cb4968..f5b0f7b 100644
--- a/Project.toml
+++ b/Project.toml
@@ -8,6 +8,7 @@ version = "0.1.0"
 Distances = "b4f34e82-e78d-54a5-968a-f98e89d6e8f7"
 NearestNeighbors = "b8a86587-4115-5ab1-83bc-aa920d37bbce"
 Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
+Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 
 [compat]
 Distances = "0.8, 0.9"
diff --git a/docs/src/dev.md b/docs/src/dev.md
index 1eddb86..4575647 100644
--- a/docs/src/dev.md
+++ b/docs/src/dev.md
@@ -54,3 +54,18 @@ to satisfy both mandatory API as well as this one.
 
 ## Insertion/deletion methods
 Simply extend `Base.insert!` and `Base.deleteat!` for your search structure.
+
+
+## Testing
+
+The [`Neighborhood.Testing`](@ref) submodule contains utilities for testing the
+return value of [`search`](@ref) and related functions for your search structure.
+These functions use `Test.@test` internally, so just call within a `@testset`
+in your unit tests.
+
+```@docs
+Neighborhood.Testing
+Neighborhood.Testing.search_allfuncs
+Neighborhood.Testing.check_search_results
+Neighborhood.Testing.test_bulksearch
+```
diff --git a/src/Neighborhood.jl b/src/Neighborhood.jl
index 14f05df..a33c64c 100644
--- a/src/Neighborhood.jl
+++ b/src/Neighborhood.jl
@@ -5,6 +5,7 @@ export Euclidean, Chebyshev, Cityblock, Minkowski
 include("api.jl")
 include("theiler.jl")
 include("kdtree.jl")
+include("Testing.jl")
 
 "Currently supported search structures"
 const SSS = [KDTree]
diff --git a/src/Testing.jl b/src/Testing.jl
new file mode 100644
index 0000000..2d05c5c
--- /dev/null
+++ b/src/Testing.jl
@@ -0,0 +1,111 @@
+"""Utilities for testing search structures."""
+module Testing
+
+using Test
+using Neighborhood
+
+export search_allfuncs, check_search_results, test_bulksearch
+
+
+"""
+Get arguments tuple to `search`, using the 3-argument version if `skip=nothing`.
+"""
+get_search_args(ss, query, t, skip) = isnothing(skip) ? (ss, query, t) : (ss, query, t, skip)
+
+
+"""
+    search_allfuncs(ss, query, t[, skip])
+
+Call [`search`](@ref)`(ss, query, t[, skip])` and check that the result matches
+those for [`isearch`](@ref) and [`knn`](@ref)/[`inrange`](@ref) (depending on
+search type) for the equivalent arguments.
+
+`skip` may be `nothing`, in which case the 3-argument methods of all functions
+will be called. Uses `Test.@test` internally.
+"""
+function search_allfuncs(ss, query, t, skip=nothing)
+    args = get_search_args(ss, query, t, skip)
+    idxs, ds = result = search(args...)
+
+    @test isearch(args...) == idxs
+    @test _alt_search_func(args...) == result
+
+    return result
+end
+
+# Call inrange() or knn() given arguments to search()
+function _alt_search_func(ss, query, t::WithinRange, args...)
+    inrange(ss, query, t.r, args...)
+end
+function _alt_search_func(ss, query, t::NeighborNumber, args...)
+    knn(ss, query, t.k, args...)
+end
+
+
+"""
+    check_search_results(data, metric, results, query, t[, skip])
+
+Check that `results = search(ss, query, t[, skip])` make sense for a search
+structure `ss` with data `data` and metric `metric`.
+
+Note that this does not calculate the known correct value to compare to (which
+may be expensive for large data sets), just that the results have the
+expected properties. `skip` may be `nothing`, in which case the 3-argument
+methods of all functions will be called. Uses `Test.@test` internally.
+
+Checks the following:
+* `results` is a 2-tuple of `(idxs, ds)`.
+* `ds` is sorted.
+* `ds[i] == metric(query, data[i])`.
+* `skip(i)` is false for all `i` in `idxs`.
+* For `t::NeighborNumber`:
+  * `length(idxs) <= t.k`.
+* For `t::WithinRange`:
+  * `d <= t.r` for all `d` in `ds`.
+"""
+function check_search_results(data, metric, results, query, t, skip=nothing)
+    idxs, ds = results
+
+    @test issorted(ds)
+    @test ds == [metric(query, data[i]) for i in idxs]
+
+    !isnothing(skip) && @test !any(map(skip, idxs))
+end
+
+function _check_search_results(data, metric, (idxs, ds), query, t::NeighborNumber, skip)
+    @test length(idxs) <= t.k
+end
+
+function _check_search_results(data, metric, (idxs, ds), query, t::WithinRange, skip)
+    @test all(<=(t.r), ds)
+end
+
+
+"""
+    test_bulksearch(ss, queries, t[, skip=nothing])
+
+Test that [`bulksearch`](@ref) gives the same results as individual applications
+of [`search`](@ref).
+
+`skip` may be `nothing`, in which case the 3-argument methods of both functions
+will be called. Uses `Test.@test` internally.
+"""
+function test_bulksearch(ss, queries, t, skip=nothing)
+    args = get_search_args(ss, queries, t, skip)
+    bidxs, bds = bulksearch(args...)
+
+    @test bulkisearch(args...) == bidxs
+
+    for (i, query) in enumerate(queries)
+        result = if isnothing(skip)
+            search(ss, query, t)
+        else
+            iskip = j -> skip(i, j)
+            search(ss, query, t, iskip)
+        end
+        @test result == (bidxs[i], bds[i])
+    end
+end
+
+
+end  # module

From e3fa4f5fc2d9b8b3e390b0764567fc45c22ff4b9 Mon Sep 17 00:00:00 2001
From: Jared Lumpe <mjlumpe@gmail.com>
Date: Tue, 7 Jul 2020 10:56:10 -0600
Subject: [PATCH 2/4] Don't expect search results to be ordered by distance

---
 docs/src/dev.md  |  1 +
 src/Testing.jl   | 33 +++++++++++++++++++++++++++------
 test/Testing.jl  | 10 ++++++++++
 test/runtests.jl |  3 +++
 4 files changed, 41 insertions(+), 6 deletions(-)
 create mode 100644 test/Testing.jl

diff --git a/docs/src/dev.md b/docs/src/dev.md
index 4575647..e029991 100644
--- a/docs/src/dev.md
+++ b/docs/src/dev.md
@@ -65,6 +65,7 @@ in your unit tests.
 
 ```@docs
 Neighborhood.Testing
+Neighborhood.Testing.cmp_search_results
 Neighborhood.Testing.search_allfuncs
 Neighborhood.Testing.check_search_results
 Neighborhood.Testing.test_bulksearch
diff --git a/src/Testing.jl b/src/Testing.jl
index 2d05c5c..6d49e78 100644
--- a/src/Testing.jl
+++ b/src/Testing.jl
@@ -4,7 +4,7 @@ module Testing
 using Test
 using Neighborhood
 
-export search_allfuncs, check_search_results, test_bulksearch
+export cmp_search_results, search_allfuncs, check_search_results, test_bulksearch
 
 
 """
@@ -13,6 +13,30 @@ Get arguments tuple to `search`, using the 3-argument version if `skip=nothing`.
 get_search_args(ss, query, t, skip) = isnothing(skip) ? (ss, query, t) : (ss, query, t, skip)
 
 
+"""
+    cmp_search_results(results...)::Bool
+
+Compare two or more sets of search results (`(idxs, ds)` tuples) and check that
+they are identical up to ordering.
+"""
+function cmp_search_results(results::Tuple{Vector, Vector}...)
+    length(results) < 2 && error("Expected at least two sets of results")
+
+    idxs1, ds1 = results[1]
+    rest = results[2:end]
+
+    idxset = Set(idxs1)
+    dist_map = Dict(i => d for (i, d) in zip(idxs1, ds1))
+
+    for (idxs_i, ds_i) in rest
+        Set(idxs_i) == idxset || return false
+        all(dist_map[i] == d for (i, d) in zip(idxs_i, ds_i)) || return false
+    end
+
+    return true
+end
+
+
 """
     search_allfuncs(ss, query, t[, skip])
 
@@ -27,8 +51,8 @@ function search_allfuncs(ss, query, t, skip=nothing)
     args = get_search_args(ss, query, t, skip)
     idxs, ds = result = search(args...)
 
-    @test isearch(args...) == idxs
-    @test _alt_search_func(args...) == result
+    @test Set(isearch(args...)) == Set(idxs)
+    cmp_search_results(result, _alt_search_func(args...))
 
     return result
 end
@@ -55,7 +79,6 @@ methods of all functions will be called. Uses `Test.@test` internally.
 
 Checks the following:
 * `results` is a 2-tuple of `(idxs, ds)`.
-* `ds` is sorted.
 * `ds[i] == metric(query, data[i])`.
 * `skip(i)` is false for all `i` in `idxs`.
 * For `t::NeighborNumber`:
@@ -65,8 +88,6 @@ Checks the following:
 """
 function check_search_results(data, metric, results, query, t, skip=nothing)
     idxs, ds = results
-
-    @test issorted(ds)
     @test ds == [metric(query, data[i]) for i in idxs]
 
     !isnothing(skip) && @test !any(map(skip, idxs))
diff --git a/test/Testing.jl b/test/Testing.jl
new file mode 100644
index 0000000..fd4e340
--- /dev/null
+++ b/test/Testing.jl
@@ -0,0 +1,10 @@
+@testset "cmp_search_results" begin
+    idxs = shuffle!(randsubseq(1:100, 0.5))
+    ds = rand(Float64, length(idxs))
+
+    p = randperm(length(idxs))
+    @test cmp_search_results((idxs, ds), (idxs[p], ds[p]))
+
+    @test !cmp_search_results((idxs, ds), (idxs[2:end], ds[2:end]))
+    @test !cmp_search_results((idxs, ds), (idxs[p], ds))
+end
diff --git a/test/runtests.jl b/test/runtests.jl
index 4b04f7a..339924f 100644
--- a/test/runtests.jl
+++ b/test/runtests.jl
@@ -1,5 +1,7 @@
 using Test, Neighborhood, StaticArrays, Random, Distances
 using Neighborhood: datatype, getmetric
+using Neighborhood.Testing
+
 
 Random.seed!(54525)
 data = [rand(SVector{3}) for i in 1:1000]
@@ -15,4 +17,5 @@ theiler2 = Theiler(2, nidxs)
 r = 0.1
 k = 5
 
+@testset "Neighborhood.Testing" begin include("Testing.jl") end
 include("nearestneighbors.jl")

From 9577fa2c075d2e92595782aac3e532df62dcbaff Mon Sep 17 00:00:00 2001
From: Jared Lumpe <mjlumpe@gmail.com>
Date: Tue, 7 Jul 2020 10:58:01 -0600
Subject: [PATCH 3/4] Add missing SearchType-specific check to
 check_search_results

---
 src/Testing.jl | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/Testing.jl b/src/Testing.jl
index 6d49e78..deb7ed4 100644
--- a/src/Testing.jl
+++ b/src/Testing.jl
@@ -89,8 +89,8 @@ Checks the following:
 function check_search_results(data, metric, results, query, t, skip=nothing)
     idxs, ds = results
     @test ds == [metric(query, data[i]) for i in idxs]
-
     !isnothing(skip) && @test !any(map(skip, idxs))
+    _check_search_results(data, metric, results, query, t, skip)
 end
 
 function _check_search_results(data, metric, (idxs, ds), query, t::NeighborNumber, skip)

From d43f29c0a996e42192fc3b6bc1014d9c4794a858 Mon Sep 17 00:00:00 2001
From: Jared Lumpe <mjlumpe@gmail.com>
Date: Tue, 7 Jul 2020 10:59:41 -0600
Subject: [PATCH 4/4] Slight rewording of docs on Testing submodule

---
 docs/src/dev.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/src/dev.md b/docs/src/dev.md
index e029991..61ded31 100644
--- a/docs/src/dev.md
+++ b/docs/src/dev.md
@@ -60,7 +60,7 @@ Simply extend `Base.insert!` and `Base.deleteat!` for your search structure.
 
 The [`Neighborhood.Testing`](@ref) submodule contains utilities for testing the
 return value of [`search`](@ref) and related functions for your search structure.
-These functions use `Test.@test` internally, so just call within a `@testset`
+Most of these functions use `Test.@test` internally, so just call within a `@testset`
 in your unit tests.
 
 ```@docs