Repository: ocsigen/lwt
Pull request #621 (2018-09-14 19:41:24)

Test lwt bytes second part
@aantron,

This part contains only two tests for now but a review could be helpfull:

- the files used to test read and write functions are named with the `.ml` extension so that they are copied in the `_build/default/test/unix` when the tests are run. I tried the `copy_files` stanza in the *test/unix/dune* file without success. Should I put the file in a dedicated directory in *src/test/unix* ?

- I had to generate a fd from the Unix module and transform it to an Lwt file descriptor. In the second test, I had to close the underlying Unix fd that I used to create the Lwt fd in order to flush everything before reading what has been written for the test. Should I close all the fd using the `Unix.close` at the end of the tests? I have the feeling that it is not really clean.

cedlemo: @aantron,

could you look at [this test](https://gist.github.com/cedlemo/ffec2ba4c461d246c461c3bb71b422a1) for the `recv` function. I created a thread for a server that listens in a unix socket and wait for the client to send a string. In another thread the client connect to the server and use the `Lwt_unix.recv` to get the string in a `Lwt_bytes.t`. My test blocks after those steps:
- server gen socket                                                                                                                                                    
- server bind socket                                                                                                                                    
- client gen socket                                                                                                                                     
- client connect socket

thanks.
aantron: I don't see the problem immediately. When you say

> - client connect socket

Do you mean that print was triggered, or that the client actually was able to connect? The print comes before the `connect` call.

To check which call is blocking (or raising a synchronous exception), you can insert prints after each call, but outside the callback:

```
let p = Lwt_unix.connect … in
prerr_endline "if connect didn't block, this print runs immediately";
p >>= fun () ->
prerr_endline "this print runs only after connect succeeds";
...
```

Also, be sure to flush your output streams. It's possible that some output is buffered, but not visible to you. `prerr_endline` flushes STDERR after each print.

Finally, I recommend not using Unix domain sockets for these tests, because Unix domain sockets will not work on Windows.
aantron: The test also looks non-deterministic: the client could run first.
cedlemo: @aantron, in the last commit I had to make some choices:

- I don't catch exception for `Lwt_unix.gethostbyname` or `Lwt_unix.socket`. 
- I use an hard coded port for the socket to use. I have read that it should be better to let the system chose the port (https://stackoverflow.com/questions/1365265/on-localhost-how-do-i-pick-a-free-port-number for example.)
- I use `Lwt_unix.sleep` in order to be sure that when the client will try to read in the socket, the server is ready. I did not find a better way with `Lwt.join`, `Lwt.choose` or `Lwt.pick`.
aantron: Sorry, comment got cut.

> I use `Lwt_unix.sleep` in order to be sure that when the client will try to read in the socket, the server is ready. I did not find a better way with `Lwt.join`, `Lwt.choose` or `Lwt.pick`.

You just need to make the client wait on a promise that will be resolved by the server. This also lets you pass the server's system-assigned port to the client.

First create the promise:

```ocaml
let server_is_ready, notify_server_is_ready = Lwt.wait () in
...
```

in the server,

```ocaml
let server_address = Lwt_unix.getsockname server_socket in
Lwt.wakeup_later server_is_ready server_address;
```


At the beginning of the client:


```ocaml
server_is_ready
>>= fun server_address ->
(* The rest of the client *)
```
cedlemo: @aantron, I have tested all the send/recv functions. Could you make a review? Do you think that I should try to refactor ?


Patch:
diff --git a/dune-project b/dune-project
index 7655de077..de4fc2092 100644
--- a/dune-project
+++ b/dune-project
@@ -1,1 +1,1 @@
-(lang dune 1.1)
+(lang dune 1.0)
diff --git a/src/unix/config/configure.ml b/src/unix/config/configure.ml
index cc956918d..9767229e4 100644
--- a/src/unix/config/configure.ml
+++ b/src/unix/config/configure.ml
@@ -35,15 +35,18 @@ let main () =
   let config_file = "lwt_config" in
 
   begin match !default_configuration with
-  | Some true ->
+  | Some true -> begin
     (* Check if we have the opam command and conf-libev is installed. If so,
-       behave as if -use-libev true was passed. *)
-    if not @@ Sys.file_exists config_file then
-      if Sys.command "opam --version > /dev/null" = 0 then
-        if Sys.command "opam list --installed conf-libev > /dev/null" = 0 then
-          use_libev := Some true
-        else
-          use_libev := Some false
+       behave as if -use-libev true was passed.
+       opam 2.0.0 returns exit code 0 for `opam list` even if the package is not available,
+       so we instead use opam config vars to detect the presence of conf-libev *)
+    try
+      let ch = Unix.open_process_in "opam config var conf-libev:installed 2>/dev/null" in
+      match input_line ch with
+      |"true" -> use_libev := Some true
+      |_ -> use_libev := Some false
+    with _ -> use_libev := Some false
+  end
   | _ ->
     ()
   end;
diff --git a/src/unix/config/dune b/src/unix/config/dune
index ad3d7de2e..31d268d50 100644
--- a/src/unix/config/dune
+++ b/src/unix/config/dune
@@ -1,6 +1,7 @@
 (executable
  (name configure)
  (modules configure)
+ (libraries unix)
  (flags (:standard)))
 
 (executable
diff --git a/src/util/travis.sh b/src/util/travis.sh
index 11ce6add9..1a81fe304 100644
--- a/src/util/travis.sh
+++ b/src/util/travis.sh
@@ -136,7 +136,7 @@ else
     LIBEV_FLAG=false
 fi
 
-ocaml src/unix/config/configure.ml -use-libev $LIBEV_FLAG
+dune exec src/unix/config/configure.exe -- -use-libev $LIBEV_FLAG
 make build
 make test
 make coverage
@@ -147,14 +147,3 @@ make coverage
 make clean
 make install-for-packaging-test
 make packaging-test
-
-
-
-# Some sanity checks.
-if [ "$LIBEV" == no ]
-then
-    ! opam list -i conf-libev
-fi
-
-opam list -i ppx_tools_versioned
-! opam list -i batteries


Test patch:
diff --git a/test/unix/bytes_io_data b/test/unix/bytes_io_data
new file mode 100644
index 000000000..0373d9336
--- /dev/null
+++ b/test/unix/bytes_io_data
@@ -0,0 +1,1 @@
+abcdef
diff --git a/test/unix/dune b/test/unix/dune
index adeed5bf7..a2abb1428 100644
--- a/test/unix/dune
+++ b/test/unix/dune
@@ -10,4 +10,6 @@
 (alias
  (name runtest)
  (package lwt)
- (action (run %{exe:main.exe})))
+ (action (run %{exe:main.exe}))
+ (deps bytes_io_data)
+)
diff --git a/test/unix/test_lwt_bytes.ml b/test/unix/test_lwt_bytes.ml
index e58de0c97..4c4644938 100644
--- a/test/unix/test_lwt_bytes.ml
+++ b/test/unix/test_lwt_bytes.ml
@@ -1,10 +1,64 @@
 (* This file is part of Lwt, released under the MIT license. See LICENSE.md for
    details, or visit https://github.com/ocsigen/lwt/blob/master/LICENSE.md. *)
 
+open Lwt.Infix
 open Test
 
 let bytes_equal (b1:Bytes.t) (b2:Bytes.t) = b1 = b2
 
+let tcp_server_client_exchange server_logic client_logic =
+  let server_is_ready, notify_server_is_ready = Lwt.wait () in
+  let server () =
+    let sock = Lwt_unix.socket Lwt_unix.PF_INET Lwt_unix.SOCK_STREAM 0 in
+    let sockaddr = Lwt_unix.ADDR_INET (Unix.inet_addr_loopback, 0) in
+    Lwt_unix.bind sock sockaddr
+    >>= fun () ->
+    let server_address = Lwt_unix.getsockname sock in
+    let () = Lwt_unix.listen sock 5 in
+    Lwt.wakeup_later notify_server_is_ready server_address;
+    Lwt_unix.accept sock
+    >>= fun (fd_client, _) ->
+    server_logic fd_client
+    >>= fun _n -> Lwt_unix.close fd_client
+    >>= fun () -> Lwt_unix.close sock
+  in
+  let client () =
+    server_is_ready
+    >>= fun sockaddr ->
+    let sock = Lwt_unix.socket Lwt_unix.PF_INET Lwt_unix.SOCK_STREAM 0 in
+    Lwt_unix.connect sock sockaddr
+    >>= fun () ->
+    client_logic sock
+    >>= fun _n -> Lwt_unix.close sock
+  in
+  Lwt.join [client (); server ()]
+
+let udp_server_client_exchange server_logic client_logic =
+  let server_is_ready, notify_server_is_ready = Lwt.wait () in
+  let server () =
+    let sock = Lwt_unix.socket Lwt_unix.PF_INET Lwt_unix.SOCK_DGRAM 0 in
+    let sockaddr = Lwt_unix.ADDR_INET (Unix.inet_addr_loopback, 0) in
+    Lwt_unix.bind sock sockaddr
+    >>= fun () ->
+    let server_address = Lwt_unix.getsockname sock in
+    Lwt.wakeup_later notify_server_is_ready server_address;
+    server_logic sock
+    >>= fun (_n, _sockaddr) -> Lwt_unix.close sock
+  in
+  let client () =
+    server_is_ready
+    >>= fun sockaddr ->
+    let sock = Lwt_unix.socket Lwt_unix.PF_INET Lwt_unix.SOCK_DGRAM 0 in
+    client_logic sock sockaddr
+    >>= fun (_n) -> Lwt_unix.close sock
+  in
+  Lwt.join [client (); server ()]
+
+let gen_buf n =
+  let buf = Lwt_bytes.create n in
+  let () = Lwt_bytes.fill buf 0 n '\x00' in
+  buf
+
 let suite = suite "lwt_bytes" [
     test "create" begin fun () ->
       let len = 5 in
@@ -370,8 +424,8 @@ let suite = suite "lwt_bytes" [
       let str = "abcdef" in
       let buf = Lwt_bytes.of_string str in
       try
-      let _ = Lwt_bytes.proxy buf (-1) 3 in
-      Lwt.return_false
+        let _ = Lwt_bytes.proxy buf (-1) 3 in
+        Lwt.return_false
       with
       | Invalid_argument _ -> Lwt.return_true
       | _ -> Lwt.return_false
@@ -381,8 +435,8 @@ let suite = suite "lwt_bytes" [
       let str = "abcdef" in
       let buf = Lwt_bytes.of_string str in
       try
-      let _ = Lwt_bytes.proxy buf 4 3 in
-      Lwt.return_false
+        let _ = Lwt_bytes.proxy buf 4 3 in
+        Lwt.return_false
       with
       | Invalid_argument _ -> Lwt.return_true
       | _ -> Lwt.return_false
@@ -392,8 +446,8 @@ let suite = suite "lwt_bytes" [
       let str = "abcdef" in
       let buf = Lwt_bytes.of_string str in
       try
-      let _ = Lwt_bytes.proxy buf 3 (-1) in
-      Lwt.return_false
+        let _ = Lwt_bytes.proxy buf 3 (-1) in
+        Lwt.return_false
       with
       | Invalid_argument _ -> Lwt.return_true
       | _ -> Lwt.return_false
@@ -496,4 +550,131 @@ let suite = suite "lwt_bytes" [
       let check = "abcaaa" = Lwt_bytes.to_string buf in
       Lwt.return check
     end;
+
+    test "bytes read" begin fun () ->
+      let test_file = "bytes_io_data" in
+      Lwt_unix.openfile test_file [O_RDONLY] 0
+      >>= fun fd ->
+      let buf = Lwt_bytes.create 6 in
+      Lwt_bytes.read fd buf 0 6
+      >>= fun _n ->
+      let check = "abcdef" = Lwt_bytes.to_string buf in
+      Lwt_unix.close fd
+      >>= fun () ->
+      Lwt.return check
+    end;
+
+    test "bytes write" begin fun () ->
+      let test_file = "bytes_io_data_write" in
+      Lwt_unix.openfile test_file [O_RDWR;O_TRUNC; O_CREAT] 0o666
+      >>= fun fd ->
+      let buf_write = Lwt_bytes.of_string "abc" in
+      Lwt_bytes.write fd buf_write 0 3
+      >>= fun _n ->
+      Lwt_unix.close fd
+      >>= fun () ->
+      Lwt_unix.openfile test_file [O_RDONLY] 0
+      >>= fun fd ->
+      let buf_read = Lwt_bytes.create 3 in
+      Lwt_bytes.read fd buf_read 0 3
+      >>= fun _n ->
+      let check = buf_write = buf_read in
+      Lwt_unix.close fd
+      >>= fun () ->
+      Lwt.return check
+    end;
+
+    test "bytes recv" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buf = gen_buf 6 in
+      let server_logic socket =
+        Lwt_unix.write_string socket "abcdefghij" 0 9
+      in
+      let client_logic socket =
+        Lwt_bytes.recv socket buf 0 6 []
+      in
+      tcp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buf in
+      Lwt.return check
+    end;
+
+    test "bytes send" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buf = gen_buf 6 in
+      let server_logic socket =
+        Lwt_bytes.send socket (Lwt_bytes.of_string "abcdef") 0 6 []
+      in
+      let client_logic socket =
+        Lwt_bytes.recv socket buf 0 6 []
+      in
+      tcp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buf in
+      Lwt.return check
+    end;
+
+    test "bytes recvfrom" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buf = gen_buf 6 in
+      let server_logic socket =
+        Lwt_bytes.recvfrom socket buf 0 6 []
+      in
+      let client_logic socket sockaddr =
+        Lwt_unix.sendto socket (Bytes.of_string "abcdefghij") 0 9 [] sockaddr
+      in
+      udp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buf in
+      Lwt.return check
+    end;
+
+    test "bytes sendto" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buf = gen_buf 6 in
+      let server_logic socket =
+        Lwt_bytes.recvfrom socket buf 0 6 []
+      in
+      let client_logic socket sockaddr =
+        let message = Lwt_bytes.of_string "abcdefghij" in
+        Lwt_bytes.sendto socket message 0 9 [] sockaddr
+      in
+      udp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buf in
+      Lwt.return check
+    end;
+
+    test "bytes recv_msg" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buffer = gen_buf 6 in
+      let offset = 0 in
+      let io_vectors = [Lwt_bytes.io_vector ~buffer ~offset ~length:6] in
+      let server_logic socket =
+        Lwt_bytes.recv_msg ~socket ~io_vectors
+      in
+      let client_logic socket sockaddr =
+        let message = Lwt_bytes.of_string "abcdefghij" in
+        Lwt_bytes.sendto socket message 0 9 [] sockaddr
+      in
+      udp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buffer in
+      Lwt.return check
+    end;
+
+    test "bytes send_msg" ~only_if:(fun () -> not Sys.win32) begin fun () ->
+      let buffer = gen_buf 6 in
+      let offset = 0 in
+      let server_logic socket =
+        let io_vectors = [Lwt_bytes.io_vector ~buffer ~offset ~length:6] in
+        Lwt_bytes.recv_msg ~socket ~io_vectors
+      in
+      let client_logic socket sockaddr =
+        Lwt_unix.connect socket sockaddr
+        >>= fun () ->
+        let message = Lwt_bytes.of_string "abcdefghij" in
+        let io_vectors = [Lwt_bytes.io_vector ~buffer:message ~offset ~length:9] in
+        Lwt_bytes.send_msg ~socket ~io_vectors ~fds:[]
+      in
+      udp_server_client_exchange server_logic client_logic
+      >>= fun () ->
+      let check = "abcdef" = Lwt_bytes.to_string buffer in
+      Lwt.return check
+    end;
   ]