From d256e8079fdbfdf26d09f3b25ceed6f9ef5b2369 Mon Sep 17 00:00:00 2001
From: Hai Nguyen Van <hai@nomadic-labs.com>
Date: Mon, 28 Sep 2020 17:42:11 +0200
Subject: [PATCH 1/3] Testing: Better comments in lib_p2p

---
 src/lib_p2p/test/test_p2p_banned_peers.ml | 20 +++++++
 src/lib_p2p/test/test_p2p_io_scheduler.ml | 13 +++++
 src/lib_p2p/test/test_p2p_ipv6set.ml      | 24 ++++++++
 src/lib_p2p/test/test_p2p_peerset.ml      | 24 ++++++++
 src/lib_p2p/test/test_p2p_pool.ml         | 71 +++++++++++++++++------
 src/lib_p2p/test/test_p2p_socket.ml       | 63 ++++++++++++++++++++
 6 files changed, 197 insertions(+), 18 deletions(-)

diff --git a/src/lib_p2p/test/test_p2p_banned_peers.ml b/src/lib_p2p/test/test_p2p_banned_peers.ml
index 1695089bcf48..f94bf1fff6ed 100644
--- a/src/lib_p2p/test/test_p2p_banned_peers.ml
+++ b/src/lib_p2p/test/test_p2p_banned_peers.ml
@@ -23,6 +23,14 @@
 (*                                                                           *)
 (*****************************************************************************)
 
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_banned_peers
+   Subject:      On banning peers and usage of Access Control Lists (ACL)
+                 using sets and POSIX time tags.
+*)
+
 include Internal_event.Legacy_logging.Make (struct
   let name = "test-p2p-banned_peers"
 end)
@@ -40,6 +48,10 @@ let baz = a ("baz", "a::2")
 
 let peers = [foo; bar; baz]
 
+(* Test.
+   In an empty ACL of size 10, nobody is tagged as blacklisted or
+   greylisted yet.
+*)
 let test_empty _ =
   let empty = P2p_acl.create 10 in
   List.iter
@@ -48,6 +60,10 @@ let test_empty _ =
     peers ;
   Lwt.return_unit
 
+(* Test.
+   From an empty ACL of size 10, peers [foo], [bar] and [baz] are
+   greylisted.
+*)
 let test_ban _ =
   let set = P2p_acl.create 10 in
   List.iter
@@ -59,6 +75,10 @@ let test_ban _ =
     peers ;
   Lwt.return_unit
 
+(* Test.
+   From an empty ACL of size 10, peers [peers] are greylisted, then
+   they are all removed.
+*)
 let test_gc _ =
   let set = P2p_acl.create 10 in
   List.iter
diff --git a/src/lib_p2p/test/test_p2p_io_scheduler.ml b/src/lib_p2p/test/test_p2p_io_scheduler.ml
index c55d10b7116c..ed19baf12850 100644
--- a/src/lib_p2p/test/test_p2p_io_scheduler.ml
+++ b/src/lib_p2p/test/test_p2p_io_scheduler.ml
@@ -24,6 +24,14 @@
 (*                                                                           *)
 (*****************************************************************************)
 
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_io_scheduler_ipv4
+   Dependencies: src/lib_p2p/test/process.ml
+   Subject:      On I/O scheduling of client-server connections.
+*)
+
 include Internal_event.Legacy_logging.Make (struct
   let name = "test-p2p-io-scheduler"
 end)
@@ -160,6 +168,11 @@ let client ?max_upload_speed ?write_queue_size addr port time _n =
   let stat = P2p_io_scheduler.stat conn in
   lwt_log_notice "Client OK %a" P2p_stat.pp stat >>= fun () -> return_unit
 
+(* Test.
+   Listens to address [addr] on port [port] to open a socket [main_socket].
+   Spawns a server on it, and [n] clients connecting to the server. Then,
+   the server will close all connections.
+*)
 let run ?display_client_stat ?max_download_speed ?max_upload_speed
     ~read_buffer_size ?read_queue_size ?write_queue_size addr port time n =
   Internal_event_unix.init ()
diff --git a/src/lib_p2p/test/test_p2p_ipv6set.ml b/src/lib_p2p/test/test_p2p_ipv6set.ml
index 48adca5964f9..78a91319d5a8 100644
--- a/src/lib_p2p/test/test_p2p_ipv6set.ml
+++ b/src/lib_p2p/test/test_p2p_ipv6set.ml
@@ -23,6 +23,13 @@
 (*                                                                           *)
 (*****************************************************************************)
 
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_ipv6set
+   Subject:      Sets of IPV6 addresses (as keys in Patricia Trees).
+*)
+
 include Internal_event.Legacy_logging.Make (struct
   let name = "test-p2p-banned_ip"
 end)
@@ -51,6 +58,9 @@ let of_list l =
     P2p_acl.IpSet.empty
     l
 
+(* Test.
+   An empty set does not contain any IP address.
+*)
 let test_empty _ =
   let addrs = List.map a ["::"; "ffff::"; "a::2"] in
   List.iter
@@ -61,6 +71,10 @@ let test_empty _ =
         (P2p_acl.IpSet.mem addr P2p_acl.IpSet.empty))
     addrs
 
+(* Test.
+   Addresses in [included] are contained in [set], while
+   [not_included] are not.
+*)
 let test_inclusion _ =
   let set =
     P2p_acl.IpSet.add_prefix (p "ffff::/16") timenow P2p_acl.IpSet.empty
@@ -105,6 +119,9 @@ let test_inclusion _ =
   assert_equal ~msg:__LOC__ false (P2p_acl.IpSet.mem (a "b111:8000::1") set) ;
   assert_equal ~msg:__LOC__ false (P2p_acl.IpSet.mem (a "1234:5678::100") set)
 
+(* Test.
+   Contiguous prefixes preserve consistency of IP sets.
+*)
 let test_contiguous _ =
   let set = of_list [p "::/1"; p "8000::/1"] in
   List.iter
@@ -116,6 +133,10 @@ module PSet = Set.Make (Ipaddr.V6.Prefix)
 let print_pset ppf pset =
   PSet.iter (fun p -> Format.fprintf ppf "%a " Ipaddr.V6.Prefix.pp p) pset
 
+(* Test.
+   A set created with [Pset.of_list] has the same elements as if it
+   was successively created with [Pset.add] from [Pset.empty].
+*)
 let test_fold _ =
   let addr_list = [p "::/1"; p "8000::/2"; p "ffff:ffff::/32"] in
   let pset = PSet.of_list addr_list in
@@ -130,6 +151,9 @@ let test_fold _ =
 let print_list ppf l =
   List.iter (fun p -> Format.fprintf ppf "%a " Ipaddr.V6.Prefix.pp p) l
 
+(* Test.
+   Casting lists to sets preserve their elements.
+*)
 let test_to_list _ =
   let to_list s = P2p_acl.IpSet.fold (fun k _v acc -> k :: acc) s [] in
   let list_eq = List.for_all2 (fun x y -> Ipaddr.V6.Prefix.compare x y = 0) in
diff --git a/src/lib_p2p/test/test_p2p_peerset.ml b/src/lib_p2p/test/test_p2p_peerset.ml
index 3876638edad1..8debdf036e16 100644
--- a/src/lib_p2p/test/test_p2p_peerset.ml
+++ b/src/lib_p2p/test/test_p2p_peerset.ml
@@ -23,6 +23,14 @@
 (*                                                                           *)
 (*****************************************************************************)
 
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_peerset
+   Subject:      On banning peers and usage of Access Control Lists (ACL)
+                 using FIFO caches filled with peers' ids.
+*)
+
 include Internal_event.Legacy_logging.Make (struct
   let name = "test-p2p-banned_peers"
 end)
@@ -31,6 +39,9 @@ let assert_equal_bool ~msg a b = if a <> b then Alcotest.fail msg
 
 let a s = P2p_peer.Id.hash_string [s]
 
+(* Test.
+   An empty ACL (built as a FIFO cache) initially contains no peer.
+*)
 let test_empty _ =
   let peers = List.map a ["foo"; "bar"; "baz"] in
   let empty = P2p_acl.PeerFIFOCache.create 10 in
@@ -42,6 +53,9 @@ let test_empty _ =
         (P2p_acl.PeerFIFOCache.mem empty peer))
     peers
 
+(* Test.
+   From an empty ACL cache, we successively add "foo", "bar", "baz".
+*)
 let test_add _ =
   let peers = List.map a ["foo"; "bar"; "baz"] in
   let set = P2p_acl.PeerFIFOCache.create 10 in
@@ -51,6 +65,11 @@ let test_add _ =
       assert_equal_bool ~msg:__LOC__ true (P2p_acl.PeerFIFOCache.mem set peer))
     peers
 
+(* Test.
+   From an empty ACL cache, we successively add "foo", "bar", "baz".
+   We test that "bar" exists, then remove it. Hence, it does not exist
+   in the cache anymore.
+*)
 let test_remove _ =
   let peers = List.map a ["foo"; "bar"; "baz"] in
   let set = P2p_acl.PeerFIFOCache.create 10 in
@@ -62,6 +81,11 @@ let test_remove _ =
     false
     (P2p_acl.PeerFIFOCache.mem set (a "bar"))
 
+(* Test.
+   An initial cache of size 3 is filled with 3 peer ids. Additionnaly,
+   we add "foo" and "zor", hence they are cached but not for "bar"
+   anymore. "baz" is the FIFO head and so on...
+*)
 let test_LRU_overflow _ =
   let peers = List.map a ["foo"; "bar"; "baz"] in
   let set = P2p_acl.PeerFIFOCache.create 3 in
diff --git a/src/lib_p2p/test/test_p2p_pool.ml b/src/lib_p2p/test/test_p2p_pool.ml
index fccb7606cce2..ec0a0bb2a874 100644
--- a/src/lib_p2p/test/test_p2p_pool.ml
+++ b/src/lib_p2p/test/test_p2p_pool.ml
@@ -24,29 +24,35 @@
 (*                                                                           *)
 (*****************************************************************************)
 
-(** Testing of the Pool
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_ipv6set
+   Dependencies: src/lib_p2p/test/process.ml
+   Subject:      Testing of the Pool
 
-    Each test launches nodes in separate process, each node has its own pool and is given a function to be executed.
+   Each test launches nodes in separate process, each node has its own
+   pool and is given a function to be executed.
 
-    [Simple] test: each node pings all other nodes.
+   - [Simple] test: each node pings all other nodes.
 
-    [Random] test: each node pings all other nodes but at random points in time.
+   - [Random] test: each node pings all other nodes but at random
+   points in time.
 
-    [Garbled] test: each node write garbled data to all peers, then it checks that connections are closed.
+   - [Garbled] test: each node write garbled data to all peers, then
+   it checks that connections are closed.
 
-    [Overcrowded] tests:
-    both test [run] and [run_mixed_versions] creates one target node which
-    knows all the other points  and has max_incoming_connections set
-    to zero,
-    all other clients know only the target and try to connect to it.
+   [Overcrowded] tests: both test [run] and [run_mixed_versions]
+   creates one target node which knows all the other points and has
+   max_incoming_connections set to zero, all other clients know only
+   the target and try to connect to it.
 
-    In [run], each client use p2p v.1 and check that they eventually
-    know all other nodes thanks to the node reply.
-
-    In [run_mixed_versions], half the clients do as in run and half of
-    the clients use p2p v.0 and check that they didn't receive new
-    nodes (meaning that [target] actually sent a Nack_v_0.
+   In [run], each client use p2p v.1 and check that they eventually
+   know all other nodes thanks to the node reply.
 
+   In [run_mixed_versions], half the clients do as in run and half of
+   the clients use p2p v.0 and check that they didn't receive new
+   nodes (meaning that [target] actually sent a Nack_v_0.
 *)
 
 include Internal_event.Legacy_logging.Make (struct
@@ -274,6 +280,10 @@ let detach_nodes ?prefix ?timeout ?min_connections ?max_connections
 
 type error += Connect | Write | Read
 
+(* Test.
+   Detaches a number of nodes (which is [!clients]). Each of them will
+   send a [Ping], then await for reading one from each other node.
+*)
 module Simple = struct
   let rec connect ~timeout connect_handler pool point =
     lwt_log_info "Connect to %a" P2p_point.Id.pp point
@@ -383,6 +393,11 @@ module Simple = struct
   let run points = detach_nodes (fun _ -> node) points
 end
 
+(* Test.
+   Detaches a number of nodes (which is [!clients]). Each of them will
+   connect to a random node to send a ping, await for a message, then
+   disconnect. This process is repeated [repeat] times.
+*)
 module Random_connections = struct
   let rec connect_random connect_handler pool total rem point n =
     Lwt_unix.sleep (0.2 +. Random.float 1.0)
@@ -426,6 +441,13 @@ module Random_connections = struct
   let run points repeat = detach_nodes (fun _ _ -> node repeat) points
 end
 
+(* Test.
+   Detaches a number of nodes. Each will connect to the nodes of the
+   pool, sync, write a dummy message, then await for reading a
+   message. Their process will finish whenever all connections are
+   closed. It is asserted that each closed connection comes with an
+   error.
+*)
 module Garbled = struct
   let is_connection_closed = function
     | Error
@@ -715,8 +737,12 @@ module Overcrowded = struct
 
   let trusted i points = if i = 0 then points else [List.hd points]
 
-  (** Running the target and the clients.
-      All clients should have their pool populated with the list of points. *)
+  (* Test.
+     Detaches a number of nodes: one of them is the target, and all
+     the rest are clients. The target will be overcrowded by the other
+     clients. All clients should have their pool populated with the
+     list of points.
+  *)
   let run points =
     (* setting connections to -1 ensure there will be no random
        acceptance of connections *)
@@ -734,6 +760,9 @@ module Overcrowded = struct
       points
       ~trusted
 
+  (* Test.
+     Same as previously but by mixing different protocol version tags.
+  *)
   let run_mixed_versions points =
     let prefix = function
       | 0 ->
@@ -759,6 +788,12 @@ module Overcrowded = struct
       ~trusted
 end
 
+(* Test.
+   Detaches a number of nodes (one target, the rest being clients) on
+   a different network. It is asserted that the connection must be
+   rejected due to the fact that the target is not on a common
+   network.
+*)
 module No_common_network = struct
   let rec connect ?iter_count ~timeout connect_handler pool point =
     lwt_log_info
diff --git a/src/lib_p2p/test/test_p2p_socket.ml b/src/lib_p2p/test/test_p2p_socket.ml
index 5c02746ebc3d..34fc70f61923 100644
--- a/src/lib_p2p/test/test_p2p_socket.ml
+++ b/src/lib_p2p/test/test_p2p_socket.ml
@@ -24,6 +24,14 @@
 (*                                                                           *)
 (*****************************************************************************)
 
+(* Testing
+   -------
+   Component:    P2P
+   Invocation:   dune build @src/lib_p2p/test/runtest_p2p_socket_ipv4
+   Dependencies: src/lib_p2p/test/process.ml
+   Subject:      Sockets and client-server communications.
+*)
+
 include Internal_event.Legacy_logging.Make (struct
   let name = "test.p2p.connection"
 end)
@@ -202,6 +210,10 @@ let is_decoding_error = function
       log_notice "Error: %a" pp_print_error err ;
       false
 
+(* Test.
+   Writing then reading through the same pipe a chunk of message [msg]
+   with encryption/decryption.
+*)
 module Crypto_test = struct
   (* maximal size of the buffer *)
   let bufsize = (1 lsl 16) - 1
@@ -308,6 +320,12 @@ module Crypto_test = struct
                Format.kasprintf Stdlib.failwith "%a" pp_print_error error))
 end
 
+(* Test.
+   Spawns a client and a server. After the client getting connected to
+   the server, it reads a message [simple_msg] sent by the server and
+   stores in [msg] of fixed same size. It asserts that both messages
+   and identical.
+*)
 module Low_level = struct
   let simple_msg = Rand.generate (1 lsl 4)
 
@@ -329,6 +347,12 @@ module Low_level = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   Spawns a client and a server. The client connects to the server
+   using identity [id2], this identity is checked on server-side. The
+   server sends a Nack message with no rejection motive. The client
+   asserts that its connection has been rejected by Nack.
+*)
 module Nack = struct
   let encoding = Data_encoding.bytes
 
@@ -362,6 +386,11 @@ module Nack = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   Spawns a client and a server. A client trying to connect to a
+   server receives and Ack message but replies with a Nack. The
+   connection is hence rejected by the client.
+*)
 module Nacked = struct
   let encoding = Data_encoding.bytes
 
@@ -381,6 +410,12 @@ module Nacked = struct
   let run _dir = return_unit
 end
 
+(* Test.
+   Spawns a client and a server. A client tries to connect to a
+   server. Both parties acknowledge. The server sends [simple_msg],
+   while the client sends [simple_msg2]. Both messages are checked for
+   consistency. Then, the connection is closed.
+*)
 module Simple_message = struct
   let encoding = Data_encoding.bytes
 
@@ -417,6 +452,12 @@ module Simple_message = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   Spawns a client and a server. A client tries to connect to a
+   server. Both parties acknowledge. The server sends [simple_msg] and
+   the client sends [simple_msg2] with a binary chunk size of 21. Both
+   messages are checked for consistency.
+*)
 module Chunked_message = struct
   let encoding = Data_encoding.bytes
 
@@ -453,6 +494,10 @@ module Chunked_message = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   Two messages of size 131072 bytes are randomly generated. After
+   successful connection, both parties send the latter messages.
+*)
 module Oversized_message = struct
   let encoding = Data_encoding.bytes
 
@@ -489,6 +534,11 @@ module Oversized_message = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   After then successful connection of a client to a server, the client
+   attempts to read a message. However, the server decides to close
+   the connection.
+*)
 module Close_on_read = struct
   let encoding = Data_encoding.bytes
 
@@ -516,6 +566,11 @@ module Close_on_read = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   After the successful connection of a client to a server, the client
+   attempts to send a message. However, the server decides to close
+   the connection.
+*)
 module Close_on_write = struct
   let encoding = Data_encoding.bytes
 
@@ -545,6 +600,14 @@ module Close_on_write = struct
   let run _dir = run_nodes client server
 end
 
+(* Test.
+   A dummy message is generated into [garbled_msg]. After the
+   successful connection of a client to a server, the server sends
+   [garbled_msg] and waits by reading a close connection is declared
+   by the client.  On the side of the client, it is asserted that the
+   message cannot be decoded.
+
+*)
 module Garbled_data = struct
   let encoding =
     let open Data_encoding in
-- 
GitLab


From adc173f10e2835d1e3f8793fc93a292dc3c49caa Mon Sep 17 00:00:00 2001
From: Hai Nguyen Van <hai@nomadic-labs.com>
Date: Mon, 5 Oct 2020 19:06:59 +0200
Subject: [PATCH 2/3] Testing: Amended according to @arvidnl's reviews

---
 src/lib_p2p/test/test_p2p_banned_peers.ml |  2 +-
 src/lib_p2p/test/test_p2p_ipv6set.ml      |  7 +--
 src/lib_p2p/test/test_p2p_peerset.ml      |  2 +-
 src/lib_p2p/test/test_p2p_pool.ml         | 60 +++++++++--------------
 4 files changed, 29 insertions(+), 42 deletions(-)

diff --git a/src/lib_p2p/test/test_p2p_banned_peers.ml b/src/lib_p2p/test/test_p2p_banned_peers.ml
index f94bf1fff6ed..09fd2df272fc 100644
--- a/src/lib_p2p/test/test_p2p_banned_peers.ml
+++ b/src/lib_p2p/test/test_p2p_banned_peers.ml
@@ -28,7 +28,7 @@
    Component:    P2P
    Invocation:   dune build @src/lib_p2p/test/runtest_p2p_banned_peers
    Subject:      On banning peers and usage of Access Control Lists (ACL)
-                 using sets and POSIX time tags.
+                 using sets and POSIX timestamps.
 *)
 
 include Internal_event.Legacy_logging.Make (struct
diff --git a/src/lib_p2p/test/test_p2p_ipv6set.ml b/src/lib_p2p/test/test_p2p_ipv6set.ml
index 78a91319d5a8..bb16bc9557de 100644
--- a/src/lib_p2p/test/test_p2p_ipv6set.ml
+++ b/src/lib_p2p/test/test_p2p_ipv6set.ml
@@ -72,8 +72,9 @@ let test_empty _ =
     addrs
 
 (* Test.
-   Addresses in [included] are contained in [set], while
-   [not_included] are not.
+   Adds address prefixes to the set, and verifies that addresses with
+   that prefix (resp. without that prefix) are included (resp. not
+   included).
 *)
 let test_inclusion _ =
   let set =
@@ -152,7 +153,7 @@ let print_list ppf l =
   List.iter (fun p -> Format.fprintf ppf "%a " Ipaddr.V6.Prefix.pp p) l
 
 (* Test.
-   Casting lists to sets preserve their elements.
+   Creating a list from a set preserves the elements of the set.
 *)
 let test_to_list _ =
   let to_list s = P2p_acl.IpSet.fold (fun k _v acc -> k :: acc) s [] in
diff --git a/src/lib_p2p/test/test_p2p_peerset.ml b/src/lib_p2p/test/test_p2p_peerset.ml
index 8debdf036e16..aad3060a2970 100644
--- a/src/lib_p2p/test/test_p2p_peerset.ml
+++ b/src/lib_p2p/test/test_p2p_peerset.ml
@@ -82,7 +82,7 @@ let test_remove _ =
     (P2p_acl.PeerFIFOCache.mem set (a "bar"))
 
 (* Test.
-   An initial cache of size 3 is filled with 3 peer ids. Additionnaly,
+   An initial cache of size 3 is filled with 3 peer ids. Additionaly,
    we add "foo" and "zor", hence they are cached but not for "bar"
    anymore. "baz" is the FIFO head and so on...
 *)
diff --git a/src/lib_p2p/test/test_p2p_pool.ml b/src/lib_p2p/test/test_p2p_pool.ml
index ec0a0bb2a874..1c89439fc070 100644
--- a/src/lib_p2p/test/test_p2p_pool.ml
+++ b/src/lib_p2p/test/test_p2p_pool.ml
@@ -30,29 +30,8 @@
    Invocation:   dune build @src/lib_p2p/test/runtest_p2p_ipv6set
    Dependencies: src/lib_p2p/test/process.ml
    Subject:      Testing of the Pool
-
-   Each test launches nodes in separate process, each node has its own
-   pool and is given a function to be executed.
-
-   - [Simple] test: each node pings all other nodes.
-
-   - [Random] test: each node pings all other nodes but at random
-   points in time.
-
-   - [Garbled] test: each node write garbled data to all peers, then
-   it checks that connections are closed.
-
-   [Overcrowded] tests: both test [run] and [run_mixed_versions]
-   creates one target node which knows all the other points and has
-   max_incoming_connections set to zero, all other clients know only
-   the target and try to connect to it.
-
-   In [run], each client use p2p v.1 and check that they eventually
-   know all other nodes thanks to the node reply.
-
-   In [run_mixed_versions], half the clients do as in run and half of
-   the clients use p2p v.0 and check that they didn't receive new
-   nodes (meaning that [target] actually sent a Nack_v_0.
+                 Each test launches nodes in separate process, each node
+                 has its own pool and is given a function to be executed.
 *)
 
 include Internal_event.Legacy_logging.Make (struct
@@ -281,8 +260,8 @@ let detach_nodes ?prefix ?timeout ?min_connections ?max_connections
 type error += Connect | Write | Read
 
 (* Test.
-   Detaches a number of nodes (which is [!clients]). Each of them will
-   send a [Ping], then await for reading one from each other node.
+   Detaches [!client] nodes. Each of them will send a [Ping] to each
+   other node, then await for reading one from each other node.
 *)
 module Simple = struct
   let rec connect ~timeout connect_handler pool point =
@@ -395,8 +374,9 @@ end
 
 (* Test.
    Detaches a number of nodes (which is [!clients]). Each of them will
-   connect to a random node to send a ping, await for a message, then
-   disconnect. This process is repeated [repeat] times.
+   connect to each other node at random points in time, to send a
+   ping, await for a message, then disconnect. This process is
+   repeated [repeat] times.
 *)
 module Random_connections = struct
   let rec connect_random connect_handler pool total rem point n =
@@ -442,11 +422,11 @@ module Random_connections = struct
 end
 
 (* Test.
-   Detaches a number of nodes. Each will connect to the nodes of the
-   pool, sync, write a dummy message, then await for reading a
-   message. Their process will finish whenever all connections are
-   closed. It is asserted that each closed connection comes with an
-   error.
+   Detaches a number of nodes. Each will connect to all other nodes
+   (peers) of the pool, sync, write garbled data, then await for
+   reading a message. Their process will finish whenever all
+   connections are closed. It is asserted that each closed connection
+   comes with an error.
 *)
 module Garbled = struct
   let is_connection_closed = function
@@ -738,10 +718,13 @@ module Overcrowded = struct
   let trusted i points = if i = 0 then points else [List.hd points]
 
   (* Test.
-     Detaches a number of nodes: one of them is the target, and all
-     the rest are clients. The target will be overcrowded by the other
-     clients. All clients should have their pool populated with the
-     list of points.
+     Detaches a number of nodes: one of them is the target (its
+     max_incoming_connections is set to zero), and all the rest are
+     clients (knowing only the target). The target will be overcrowded
+     by the other clients connecting to it. All clients should have
+     their pool populated with the list of points. Specifically for
+     this test function, each client use p2p v.1 and check that they
+     eventually know all other nodes thanks to the node reply.
   *)
   let run points =
     (* setting connections to -1 ensure there will be no random
@@ -761,7 +744,10 @@ module Overcrowded = struct
       ~trusted
 
   (* Test.
-     Same as previously but by mixing different protocol version tags.
+     Same as previously but by mixing different protocol version
+     tags. Half the clients do as in run and half of the clients use
+     p2p v.0 and check that they didn't receive new nodes (meaning
+     that [target] actually sent a Nack_v_0.
   *)
   let run_mixed_versions points =
     let prefix = function
-- 
GitLab


From 072e77de09bcc7d4c4f12c25101e10aa07e5b640 Mon Sep 17 00:00:00 2001
From: Hai Nguyen Van <hai@nomadic-labs.com>
Date: Wed, 7 Oct 2020 14:19:39 +0200
Subject: [PATCH 3/3] Testing: Minor changes by @julien.t

---
 src/lib_p2p/test/test_p2p_banned_peers.ml | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/lib_p2p/test/test_p2p_banned_peers.ml b/src/lib_p2p/test/test_p2p_banned_peers.ml
index 09fd2df272fc..dd7ed3df9a14 100644
--- a/src/lib_p2p/test/test_p2p_banned_peers.ml
+++ b/src/lib_p2p/test/test_p2p_banned_peers.ml
@@ -76,8 +76,10 @@ let test_ban _ =
   Lwt.return_unit
 
 (* Test.
-   From an empty ACL of size 10, peers [peers] are greylisted, then
-   they are all removed.
+   From an empty ACL of size 10, peers [peers] are greylisted since
+   epoch, then a garbage collection is triggered on the table at the
+   [Ptime.max] date (e.g., far in the future).  All point should have
+   been removed.
 *)
 let test_gc _ =
   let set = P2p_acl.create 10 in
-- 
GitLab