From bfde9f6d59986424efd33f9414a2bd676eb85d17 Mon Sep 17 00:00:00 2001
From: kernelmethod <17100608+kernelmethod@users.noreply.github.com>
Date: Sat, 6 Jun 2020 14:44:08 -0600
Subject: [PATCH 1/4] Add a second type parameter to the Skiplist and
 SkiplistNode types which will be used to indicate whether the Skiplist is a
 list or a set.

---
 src/core.jl       |  8 ++++----
 src/list.jl       | 38 +++++++++++++++++++-------------------
 src/node.jl       | 24 ++++++++++++------------
 test/test_node.jl | 34 +++++++++++++++++-----------------
 4 files changed, 52 insertions(+), 52 deletions(-)

diff --git a/src/core.jl b/src/core.jl
index 624a92b..79b9796 100644
--- a/src/core.jl
+++ b/src/core.jl
@@ -21,7 +21,7 @@ const IS_SENTINEL = FLAG_IS_LEFT_SENTINEL | FLAG_IS_RIGHT_SENTINEL
 Typedefs
 ===========================#
 
-mutable struct SkiplistNode{T}
+mutable struct SkiplistNode{T,M}
     val :: T
     next :: Vector{SkiplistNode{T}}
     marked_for_deletion :: Bool
@@ -30,7 +30,7 @@ mutable struct SkiplistNode{T}
     lock :: ReentrantLock
 end
 
-struct Skiplist{T}
+struct Skiplist{T,M}
     height_p :: Float64
     max_height :: Int64
 
@@ -40,8 +40,8 @@ struct Skiplist{T}
     length :: Atomic{Int64}
 end
 
-abstract type LeftSentinel{T} end
-abstract type RightSentinel{T} end
+abstract type LeftSentinel{T,M} end
+abstract type RightSentinel{T,M} end
 
 #===========================
 Simple function definitions
diff --git a/src/list.jl b/src/list.jl
index 10cafd8..2f41a95 100644
--- a/src/list.jl
+++ b/src/list.jl
@@ -11,15 +11,17 @@ using Logging
 Constructors
 ===========================#
 
-function Skiplist{T}(; max_height = DEFAULT_MAX_HEIGHT, p = DEFAULT_P) where T
-    left_sentinel = LeftSentinel{T}(; max_height=max_height)
-    right_sentinel = RightSentinel{T}(; max_height=max_height)
+Skiplist{T}(args...; kws...) where T = Skiplist{T,:List}(args...; kws...)
+
+function Skiplist{T,M}(; max_height = DEFAULT_MAX_HEIGHT, p = DEFAULT_P) where {T,M}
+    left_sentinel = LeftSentinel{T,M}(; max_height=max_height)
+    right_sentinel = RightSentinel{T,M}(; max_height=max_height)
 
     for ii = 1:max_height
         link_nodes!(left_sentinel, right_sentinel, ii)
     end
 
-    Skiplist{T}(
+    Skiplist{T,M}(
         p,
         max_height,
         left_sentinel,
@@ -125,8 +127,8 @@ function Base.in(val, list :: Skiplist)
         !is_marked_for_deletion(successors[level_found])
 end
 
-Base.insert!(list :: Skiplist, val) =
-    insert!(list, SkiplistNode(val; p=list.height_p, max_height=list.max_height))
+Base.insert!(list :: Skiplist{_,M}, val) where {_,M} =
+    insert!(list, SkiplistNode{M}(val; p=list.height_p, max_height=list.max_height))
 
 function Base.insert!(list :: Skiplist, node :: SkiplistNode)
     while true
@@ -215,26 +217,24 @@ Skiplist internal API
 ===========================#
 
 function find_node(list :: Skiplist{T}, val) where T
-    h = height(list)
-    predecessors = Vector{SkiplistNode{T}}(undef, h)
-    successors = Vector{SkiplistNode{T}}(undef, h)
+    predecessors = Vector{SkiplistNode{T}}(undef, height(list))
+    successors = Vector{SkiplistNode{T}}(undef, height(list))
 
     layer_found = -1
 
     current_node = list.left_sentinel
-    ii = h
-    while ii > 0
+    for ii = height(list):-1:1
         next_node = next(current_node, ii)
-        if next_node < val
+        while next_node < val
             current_node = next_node
-        else
-            if layer_found == -1 && next_node == val
-                layer_found = ii
-            end
-            predecessors[ii] = current_node
-            successors[ii] = next_node
-            ii -= 1
+            next_node = next(current_node, ii)
+        end
+
+        if layer_found == -1 && next_node == val
+            layer_found = ii
         end
+        predecessors[ii] = current_node
+        successors[ii] = next_node
     end
 
     layer_found, predecessors, successors
diff --git a/src/node.jl b/src/node.jl
index d002553..de008d9 100644
--- a/src/node.jl
+++ b/src/node.jl
@@ -10,27 +10,27 @@ using Base.Threads
 Constructors
 ===========================#
 
-SkiplistNode(val :: T; kws...) where T =
-    SkiplistNode{T}(val; kws...)
+SkiplistNode{M}(val :: T; kws...) where {T,M} =
+    SkiplistNode{T,M}(val; kws...)
 
-SkiplistNode(val :: T, height; kws...) where T =
-    SkiplistNode{T}(val, height; kws...)
+SkiplistNode{M}(val :: T, height; kws...) where {T,M} =
+    SkiplistNode{T,M}(val, height; kws...)
 
-SkiplistNode{T}(val; p = DEFAULT_P, max_height = DEFAULT_MAX_HEIGHT, kws...) where T =
-    SkiplistNode{T}(val, random_height(p; max_height=max_height); kws...)
+SkiplistNode{T,M}(val; p = DEFAULT_P, max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M} =
+    SkiplistNode{T,M}(val, random_height(p; max_height=max_height); kws...)
 
-function SkiplistNode{T}(val, height; flags = 0x0, max_height = DEFAULT_MAX_HEIGHT) where T
+function SkiplistNode{T,M}(val, height; flags = 0x0, max_height = DEFAULT_MAX_HEIGHT) where {T,M}
     height = min(height, max_height)
     next = Vector{SkiplistNode{T}}(undef, height)
     lock = ReentrantLock()
 
-    SkiplistNode{T}(val, next, false, false, flags, lock)
+    SkiplistNode{T,M}(val, next, false, false, flags, lock)
 end
 
-LeftSentinel{T}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where T =
-    SkiplistNode{T}(zero(T), max_height; flags = FLAG_IS_LEFT_SENTINEL, kws...)
-RightSentinel{T}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where T =
-    SkiplistNode{T}(zero(T), max_height; flags = FLAG_IS_RIGHT_SENTINEL, kws...)
+LeftSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M} =
+    SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_LEFT_SENTINEL, kws...)
+RightSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M} =
+    SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_RIGHT_SENTINEL, kws...)
 
 #===========================
 External API
diff --git a/test/test_node.jl b/test/test_node.jl
index 476bc37..f0940ad 100644
--- a/test/test_node.jl
+++ b/test/test_node.jl
@@ -11,15 +11,15 @@ using Skiplists: SkiplistNode, LeftSentinel, RightSentinel
     Random.seed!(0)
 
     @testset "Construct SkiplistNode" begin
-        node = SkiplistNode(1)
-        @test isa(node, SkiplistNode{Int64})
+        node = SkiplistNode{:List}(1)
+        @test isa(node, SkiplistNode{Int64,:List})
         @test height(node) > 0
         @test Skiplists.key(node) == 1
 
-        left_sentinel = LeftSentinel{Int64}()
-        right_sentinel = RightSentinel{Int64}()
-        @test isa(left_sentinel, SkiplistNode{Int64})
-        @test isa(right_sentinel, SkiplistNode{Int64})
+        left_sentinel = LeftSentinel{Int64,:List}()
+        right_sentinel = RightSentinel{Int64,:List}()
+        @test isa(left_sentinel, SkiplistNode{Int64,:List})
+        @test isa(right_sentinel, SkiplistNode{Int64,:List})
         @test Skiplists.is_sentinel(left_sentinel) && Skiplists.is_left_sentinel(left_sentinel)
         @test Skiplists.is_sentinel(right_sentinel) && Skiplists.is_right_sentinel(right_sentinel)
         @test Skiplists.height(left_sentinel) == Skiplists.DEFAULT_MAX_HEIGHT
@@ -32,17 +32,17 @@ using Skiplists: SkiplistNode, LeftSentinel, RightSentinel
 
         # If two integers are passed to the SkiplistNode constructor, the second
         # integer should be used as the node's height.
-        @test SkiplistNode(1, 30) |> height == 30
-        @test SkiplistNode(1, 100; max_height=50) |> height == 50
+        @test SkiplistNode{:List}(1, 30) |> height == 30
+        @test SkiplistNode{:List}(1, 100; max_height=50) |> height == 50
     end
 
     @testset "Compare SkiplistNode pairs" begin
-        node_1 = SkiplistNode(typemin(Int64))
-        node_2 = SkiplistNode(-1)
-        node_3 = SkiplistNode(1)
-        node_4 = SkiplistNode(typemax(Int64))
-        left_sentinel = LeftSentinel{Int64}()
-        right_sentinel = RightSentinel{Int64}()
+        node_1 = SkiplistNode{:List}(typemin(Int64))
+        node_2 = SkiplistNode{:List}(-1)
+        node_3 = SkiplistNode{:List}(1)
+        node_4 = SkiplistNode{:List}(typemax(Int64))
+        left_sentinel = LeftSentinel{Int64,:List}()
+        right_sentinel = RightSentinel{Int64,:List}()
 
         @test left_sentinel ≤ node_1 && !(node_1 ≤ left_sentinel)
         @test node_1 ≤ node_2 && !(node_2 ≤ node_1)
@@ -52,14 +52,14 @@ using Skiplists: SkiplistNode, LeftSentinel, RightSentinel
     end
 
     @testset "Link SkiplistNodes" begin
-        node_1 = SkiplistNode(0)
-        node_2 = SkiplistNode(1)
+        node_1 = SkiplistNode{:List}(0)
+        node_2 = SkiplistNode{:List}(1)
 
         Skiplists.link_nodes!(node_1, node_2, 1)
         @test Skiplists.next(node_1, 1) == node_2
     end
 
     @testset "Check SkiplistNode deletability" begin
-        node = SkiplistNode(1)
+        node = SkiplistNode{:List}(1)
     end
 end

From 290e0996d1520259752373b3d15da1e3896efb0b Mon Sep 17 00:00:00 2001
From: kernelmethod <17100608+kernelmethod@users.noreply.github.com>
Date: Sat, 6 Jun 2020 15:00:12 -0600
Subject: [PATCH 2/4] Add a SkiplistSet type alias, and ensure that Skiplist
 can only be created in modes :List and :Set.

---
 src/core.jl       | 2 ++
 src/list.jl       | 6 ++++++
 test/test_list.jl | 9 ++++++++-
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/src/core.jl b/src/core.jl
index 79b9796..acde414 100644
--- a/src/core.jl
+++ b/src/core.jl
@@ -43,6 +43,8 @@ end
 abstract type LeftSentinel{T,M} end
 abstract type RightSentinel{T,M} end
 
+SkiplistSet{T} = Skiplist{T,:Set}
+
 #===========================
 Simple function definitions
 ===========================#
diff --git a/src/list.jl b/src/list.jl
index 2f41a95..6d39591 100644
--- a/src/list.jl
+++ b/src/list.jl
@@ -14,6 +14,12 @@ Constructors
 Skiplist{T}(args...; kws...) where T = Skiplist{T,:List}(args...; kws...)
 
 function Skiplist{T,M}(; max_height = DEFAULT_MAX_HEIGHT, p = DEFAULT_P) where {T,M}
+    if M != :List && M != :Set
+        "Skiplist mode $M is not recognized. Valid options are :List and :Set" |>
+        ErrorException |>
+        throw
+    end
+
     left_sentinel = LeftSentinel{T,M}(; max_height=max_height)
     right_sentinel = RightSentinel{T,M}(; max_height=max_height)
 
diff --git a/test/test_list.jl b/test/test_list.jl
index a951167..e1681cd 100644
--- a/test/test_list.jl
+++ b/test/test_list.jl
@@ -1,6 +1,6 @@
 #=======================================================
 
-Tests for the Skiplist type
+Tests for the Skiplist and SkiplistSet types
 
 =======================================================#
 
@@ -13,6 +13,10 @@ using Random, Skiplists, Test
         list = Skiplist{Int64}()
         @test height(list) == 1
         @test length(list) == 0
+
+        # An error should be raised if we attempt to construct a Skiplist in an
+        # invalid mode
+        @test_throws ErrorException Skiplist{Int64,:Foo}()
     end
 
     @testset "Insert into Skiplist" begin
@@ -106,3 +110,6 @@ using Random, Skiplists, Test
         @test vec(list) == []
     end
 end
+
+@testset "SkiplistSet tests" begin
+end

From 8bb222e498923cd14ef140e31c5ea3021855a49c Mon Sep 17 00:00:00 2001
From: kernelmethod <17100608+kernelmethod@users.noreply.github.com>
Date: Sat, 6 Jun 2020 15:44:04 -0600
Subject: [PATCH 3/4] Bugfixes and improvements for Skiplist and SkiplistSet.

- Ensure that sentinel nodes are marked as fully linked.
- Stop using vec() to convert a Skiplist into a Vector. From now on,
  collect() should be used (in line with other iterators in Julia).
- Add an implementation of Base.insert! for SkiplistSet.
---
 src/Skiplists.jl              |   2 +-
 src/list.jl                   | 112 ++++++++++++++++++----------------
 src/node.jl                   |  29 ++++++---
 test/test_list.jl             |  46 +++++++++++---
 test/test_list_concurrency.jl |  10 +--
 test/test_node.jl             |   4 ++
 6 files changed, 131 insertions(+), 72 deletions(-)

diff --git a/src/Skiplists.jl b/src/Skiplists.jl
index be3602a..989605b 100644
--- a/src/Skiplists.jl
+++ b/src/Skiplists.jl
@@ -12,6 +12,6 @@ include("list.jl")
 Exports
 ===========================#
 
-export Skiplist, height
+export Skiplist, SkiplistSet, height
 
 end # module
diff --git a/src/list.jl b/src/list.jl
index 6d39591..28d77cf 100644
--- a/src/list.jl
+++ b/src/list.jl
@@ -50,13 +50,13 @@ macro validate(predecessors, successors, node, expr, type = :(:strong))
     #
     # Weak validation (used by Base.remove!) drops the second condition, since
     # the successor is supposed to be marked for deletion.
-    local check_valid = if eval(type) == :strong
+    local check_valid = if type == :(:strong)
         :(!is_marked_for_deletion(pred) &&
           !is_marked_for_deletion(succ) &&
-          next(pred, level) == succ)
-    elseif eval(type) == :weak
+          next(pred, level) === succ)
+    elseif type == :(:weak)
         :(!is_marked_for_deletion(pred) &&
-          next(pred, level) == succ)
+          next(pred, level) === succ)
     else
         "Validation type '$(type)' is not defined" |>
         ErrorException |>
@@ -106,25 +106,6 @@ Base.string(list :: Skiplist) = "Skiplist(length = $(length(list)), height = $(h
 Base.show(list :: Skiplist) = println(string(list))
 Base.display(list :: Skiplist) = println(string(list))
 
-"""
-    vec(list :: Skiplist{T}) where T
-
-Convert the skip list `list` into a one-dimensional `Vector{T}` containing all of the elements
-of the skip list, sorted in ascending order.
-"""
-function Base.vec(list :: Skiplist{T}) where T
-    results = Vector{T}(undef, 0)
-    current_node = list.left_sentinel
-    current_node = next(current_node, 1)
-
-    while !is_right_sentinel(current_node)
-        push!(results, current_node.val)
-        current_node = next(current_node, 1)
-    end
-
-    results
-end
-
 function Base.in(val, list :: Skiplist)
     level_found, predecessors, successors = find_node(list, val)
 
@@ -136,34 +117,63 @@ end
 Base.insert!(list :: Skiplist{_,M}, val) where {_,M} =
     insert!(list, SkiplistNode{M}(val; p=list.height_p, max_height=list.max_height))
 
-function Base.insert!(list :: Skiplist, node :: SkiplistNode)
-    while true
-        level_found, predecessors, successors = find_node(list, node)
-
-        # Update the list height.
-        #
-        # If the height of the list is greater than the old height, then we
-        # will need to replace the connections between the left and right
-        # sentinel nodes.
-        old_height = atomic_max!(list.height, height(node))
-        for ii = old_height+1:height(node)
-            push!(predecessors, list.left_sentinel)
-            push!(successors, list.right_sentinel)
+@generated function Base.insert!(list :: Skiplist{T,M}, node :: SkiplistNode) where {T,M}
+    local check_exists = if M == :Set
+        quote
+            if level_found != -1
+                node_found = successors[level_found]
+
+                # If the node is in the process of being deleted, wait until it
+                # is deleted before performing insertion again
+                if is_marked_for_deletion(node)
+                    # TODO: use Event or Condition to wait until node is deleted?
+                    continue
+                end
+
+                # If the node is _not_ in the process of being deleted, we wait
+                # until it's fully linked before we return.
+                while !is_fully_linked(node_found)
+                    # TODO: use Event or Condition instead of spinning
+                    sleep(0.001)
+                end
+                return false
+            end
         end
+    else
+        :()
+    end
 
-        # Acquire locks to predecessor nodes to ensure that they're still
-        # connected to their corresponding successors
-        valid = @validate(predecessors, successors, node, begin
-            for ii = 1:height(node)
-                link_nodes!(predecessors[ii], node, ii)
-                link_nodes!(node, successors[ii], ii)
+    quote
+        while true
+            level_found, predecessors, successors = find_node(list, node)
+
+            $check_exists
+
+            # Update the list height.
+            #
+            # If the height of the list is greater than the old height, then we
+            # will need to replace the connections between the left and right
+            # sentinel nodes.
+            old_height = atomic_max!(list.height, height(node))
+            for ii = old_height+1:height(node)
+                push!(predecessors, list.left_sentinel)
+                push!(successors, list.right_sentinel)
             end
-            mark_fully_linked!(node)
-        end)
 
-        if valid
-            atomic_add!(list.length, 1)
-            break
+            # Acquire locks to predecessor nodes to ensure that they're still
+            # connected to their corresponding successors
+            valid = @validate(predecessors, successors, node, begin
+                for ii = 1:height(node)
+                    link_nodes!(predecessors[ii], node, ii)
+                    link_nodes!(node, successors[ii], ii)
+                end
+                mark_fully_linked!(node)
+            end)
+
+            if valid
+                atomic_add!(list.length, 1)
+                break
+            end
         end
     end
 end
@@ -226,7 +236,7 @@ function find_node(list :: Skiplist{T}, val) where T
     predecessors = Vector{SkiplistNode{T}}(undef, height(list))
     successors = Vector{SkiplistNode{T}}(undef, height(list))
 
-    layer_found = -1
+    level_found = -1
 
     current_node = list.left_sentinel
     for ii = height(list):-1:1
@@ -236,12 +246,12 @@ function find_node(list :: Skiplist{T}, val) where T
             next_node = next(current_node, ii)
         end
 
-        if layer_found == -1 && next_node == val
-            layer_found = ii
+        if level_found == -1 && next_node == val
+            level_found = ii
         end
         predecessors[ii] = current_node
         successors[ii] = next_node
     end
 
-    layer_found, predecessors, successors
+    level_found, predecessors, successors
 end
diff --git a/src/node.jl b/src/node.jl
index de008d9..174afff 100644
--- a/src/node.jl
+++ b/src/node.jl
@@ -27,10 +27,17 @@ function SkiplistNode{T,M}(val, height; flags = 0x0, max_height = DEFAULT_MAX_HE
     SkiplistNode{T,M}(val, next, false, false, flags, lock)
 end
 
-LeftSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M} =
-    SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_LEFT_SENTINEL, kws...)
-RightSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M} =
-    SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_RIGHT_SENTINEL, kws...)
+function LeftSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M}
+    node = SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_LEFT_SENTINEL, kws...)
+    mark_fully_linked!(node)
+    node
+end
+
+function RightSentinel{T,M}(; max_height = DEFAULT_MAX_HEIGHT, kws...) where {T,M}
+    node = SkiplistNode{T,M}(zero(T), max_height; flags = FLAG_IS_RIGHT_SENTINEL, kws...)
+    mark_fully_linked!(node)
+    node
+end
 
 #===========================
 External API
@@ -46,8 +53,13 @@ External API
 @inline mark_for_deletion!(node) = (node.marked_for_deletion = true)
 @inline mark_fully_linked!(node) = (node.fully_linked = true)
 
-Base.string(node :: SkiplistNode) =
-    "SkiplistNode($(key(node)), height = $(height(node)))"
+function Base.string(node :: SkiplistNode)
+    result = "key = $(key(node)), height = $(height(node)), "
+    result *= "marked_for_deletion = $(is_marked_for_deletion(node)), "
+    result *= "fully_linked = $(is_fully_linked(node))"
+    "SkiplistNode($result)"
+end
+
 Base.show(node :: SkiplistNode) = println(string(node))
 Base.display(node :: SkiplistNode) = println(string(node))
 
@@ -87,7 +99,10 @@ end
 
 Base.:(==)(node :: SkiplistNode, val) = is_sentinel(node) ? false : key(node) == val
 Base.:(==)(val, node :: SkiplistNode) = (node == val)
-Base.:(==)(node_1 :: SkiplistNode, node_2 :: SkiplistNode) = (node_1 === node_2)
+Base.:(==)(node_1 :: SkiplistNode, node_2 :: SkiplistNode) =
+    (is_sentinel(node_1) || is_sentinel(node_2)) ?
+    false :
+    key(node_1) == key(node_2)
 
 # Node links
 
diff --git a/test/test_list.jl b/test/test_list.jl
index e1681cd..1019fc2 100644
--- a/test/test_list.jl
+++ b/test/test_list.jl
@@ -26,7 +26,7 @@ using Random, Skiplists, Test
             insert!(list, ii)
         end
 
-        @test vec(list) == collect(1:20)
+        @test collect(list) == collect(1:20)
         @test length(list) == 20
 
         # Insert shuffled values
@@ -35,8 +35,18 @@ using Random, Skiplists, Test
             insert!(list, ii)
         end
 
-        @test vec(list) == collect(1:20)
+        @test collect(list) == collect(1:20)
         @test length(list) == 20
+
+        # All of the nodes should be marked as 'fully linked'
+        current_node = list.left_sentinel
+        success = Skiplists.is_fully_linked(current_node)
+        while success && !Skiplists.is_right_sentinel(current_node)
+            current_node = Skiplists.next(current_node, 1)
+            success = Skiplists.is_fully_linked(current_node)
+        end
+
+        @test success
     end
 
     @testset "Iterate over Skiplist" begin
@@ -71,19 +81,19 @@ using Random, Skiplists, Test
         delete!(list, 1)
         @test length(list) == 2
         @test 1 ∉ list
-        @test vec(list) == collect(2:3)
+        @test collect(list) == collect(2:3)
 
         delete!(list, 2)
         @test length(list) == 1
         @test 2 ∉ list
-        @test vec(list) == collect(3:3)
+        @test collect(list) == collect(3:3)
 
         delete!(list, 3)
         @test length(list) == 0
         @test 3 ∉ list
 
         delete!(list, 0)
-        @test vec(list) == []
+        @test collect(list) == []
         @test length(list) == 0
     end
 
@@ -95,21 +105,41 @@ using Random, Skiplists, Test
         end
 
         @test length(list) == 4
-        @test vec(list) == [1, 1, 2, 2]
+        @test collect(list) == [1, 1, 2, 2]
         @test 1 ∈ list && 2 ∈ list
 
         delete!(list, 1)
         delete!(list, 2)
         @test length(list) == 2
-        @test vec(list) == [1, 2]
+        @test collect(list) == [1, 2]
         @test 1 ∈ list && 2 ∈ list
 
         delete!(list, 1)
         delete!(list, 2)
         @test length(list) == 0
-        @test vec(list) == []
+        @test collect(list) == []
     end
 end
 
 @testset "SkiplistSet tests" begin
+    @testset "Insert into SkiplistSet" begin
+        set = SkiplistSet{Int64}()
+        for ii = 1:10
+            insert!(set, ii)
+        end
+
+        @test length(set) == 10
+        @test collect(set) == 1:10
+
+        # If we now try to insert a duplicate element into the set, it shouldn't
+        # have any effect
+        for ii = 1:10
+            insert!(set, ii)
+        end
+
+        @test length(set) == 10
+        @test collect(set) == 1:10
+    end
 end
+
+
diff --git a/test/test_list_concurrency.jl b/test/test_list_concurrency.jl
index 2eb1bf6..0ba8810 100644
--- a/test/test_list_concurrency.jl
+++ b/test/test_list_concurrency.jl
@@ -32,13 +32,13 @@ using Base.Threads: @spawn
         wait.(tasks)
         @test length(list) == length(orig)
 
-        vl = vec(list)
+        vl = collect(list)
         @test length(vl) == length(orig)
 
         success = (vl .== sort(orig))
         if !all(success)
             @error "Failed Skiplist insertion tests"
-            @error "vec(list) != sort(orig) in the following indices: $(eachindex(vl)[success])"
+            @error "collect(list) != sort(orig) in the following indices: $(eachindex(vl)[success])"
         end
 
         @test all(success)
@@ -52,7 +52,7 @@ using Base.Threads: @spawn
             insert!(list, ii)
         end
 
-        @test vec(list) == orig
+        @test collect(list) == orig
 
         # Delete all of the even numbers in separate threads
         to_delete = filter(iseven, shuffle(orig))
@@ -70,7 +70,7 @@ using Base.Threads: @spawn
         end
 
         wait.(tasks)
-        @test vec(list) == filter(isodd, orig)
+        @test collect(list) == filter(isodd, orig)
         @test length(list) == filter(isodd, orig) |> length
     end
 
@@ -109,7 +109,7 @@ using Base.Threads: @spawn
 
         wait.(tasks)
         @test length(list) == length(expected)
-        @test vec(list) == expected
+        @test collect(list) == expected
     end
 end
 
diff --git a/test/test_node.jl b/test/test_node.jl
index f0940ad..aaab357 100644
--- a/test/test_node.jl
+++ b/test/test_node.jl
@@ -34,6 +34,10 @@ using Skiplists: SkiplistNode, LeftSentinel, RightSentinel
         # integer should be used as the node's height.
         @test SkiplistNode{:List}(1, 30) |> height == 30
         @test SkiplistNode{:List}(1, 100; max_height=50) |> height == 50
+
+        # Newly constructed sentinels should be marked as fully linked
+        @test Skiplists.is_fully_linked(left_sentinel)
+        @test Skiplists.is_fully_linked(right_sentinel)
     end
 
     @testset "Compare SkiplistNode pairs" begin

From 14ac42a7517cccccfc5eb61b63857a1831cc3c2d Mon Sep 17 00:00:00 2001
From: kernelmethod <17100608+kernelmethod@users.noreply.github.com>
Date: Sat, 6 Jun 2020 15:54:46 -0600
Subject: [PATCH 4/4] Add tests for SkiplistSet removal and set membership.

---
 test/test_list.jl | 36 ++++++++++++++++++++++++++++++++++--
 1 file changed, 34 insertions(+), 2 deletions(-)

diff --git a/test/test_list.jl b/test/test_list.jl
index 1019fc2..8f200f4 100644
--- a/test/test_list.jl
+++ b/test/test_list.jl
@@ -122,6 +122,8 @@ using Random, Skiplists, Test
 end
 
 @testset "SkiplistSet tests" begin
+    Random.seed!(0)
+
     @testset "Insert into SkiplistSet" begin
         set = SkiplistSet{Int64}()
         for ii = 1:10
@@ -133,13 +135,43 @@ end
 
         # If we now try to insert a duplicate element into the set, it shouldn't
         # have any effect
-        for ii = 1:10
+        for ii = shuffle(1:10)
             insert!(set, ii)
         end
 
         @test length(set) == 10
         @test collect(set) == 1:10
     end
-end
 
+    @testset "Remove from SkiplistSet" begin
+        set = SkiplistSet{Int64}()
+        orig = 1:100
+
+        for ii in shuffle(orig)
+            # Insert every element twice
+            insert!(set, ii)
+            insert!(set, ii)
+        end
+
+        @test length(set) == length(orig)
+        @test collect(set) == sort(orig)
+
+        # Remove all of the even elements
+        to_remove = filter(iseven, orig)
+        remaining = filter(isodd, orig) |> sort
+        for ii in shuffle(to_remove)
+            delete!(set, ii)
+            delete!(set, ii)
+        end
 
+        @test length(set) == length(remaining)
+        @test collect(set) == remaining
+
+        # Test membership of remaining elements
+        success = true
+        for ii in remaining
+            success = success && ii ∈ set
+        end
+        @test success
+    end
+end