Merge branch 'hans/ssh/cuddle_tests' into maint

* hans/ssh/cuddle_tests:
  ssh: Insert missing ssh:stop() etc in test suites
  ssh: Fix acceptor sup testcase
  ssh: Fix cli testcase
  ssh: Misspelling in test suite
  ssh: Put common user_dir setup in ssh_options_SUITE:init_per_testcases/2

6 files changed, 53 insertions, 88 deletions

diff --git a/lib/ssh/test/ssh_basic_SUITE.erl b/lib/ssh/test/ssh_basic_SUITE.erl
index 202b0afe57..d16b169da3 100644
--- a/lib/ssh/test/ssh_basic_SUITE.erl
+++ b/lib/ssh/test/ssh_basic_SUITE.erl
@@ -764,11 +764,11 @@ cli(Config) when is_list(Config) ->
     
     {ok, ChannelId} = ssh_connection:session_channel(ConnectionRef, infinity),
     ssh_connection:shell(ConnectionRef, ChannelId),
-    ok = ssh_connection:send(ConnectionRef, ChannelId, <<"q">>),
+    ssh_connection:send(ConnectionRef, ChannelId, <<"q">>),
     receive 
 	{ssh_cm, ConnectionRef,
 	 {data,0,0, <<"\r\nYou are accessing a dummy, type \"q\" to exit\r\n\n">>}} ->
-	    ok = ssh_connection:send(ConnectionRef, ChannelId, <<"q">>)
+	    ssh_connection:send(ConnectionRef, ChannelId, <<"q">>)
     after 
 	30000 -> ct:fail("timeout ~p:~p",[?MODULE,?LINE])
     end,
@@ -1491,7 +1491,7 @@ new_do_shell(IO, N, Ops=[{Order,Arg}|More]) ->
 		    ct:fail("*** Expected ~p, but got ~p",[string:strip(ExpStr),RecStr])
 	    end
     after 30000 ->
-	    ct:log("Meassage queue of ~p:~n~p",
+	    ct:log("Message queue of ~p:~n~p",
 		   [self(), erlang:process_info(self(), messages)]),
 	    case Order of
 		expect -> ct:fail("timeout, expected ~p",[string:strip(Arg)]);
diff --git a/lib/ssh/test/ssh_compat_SUITE.erl b/lib/ssh/test/ssh_compat_SUITE.erl
index 82b83dd83d..f7eda1dc08 100644
--- a/lib/ssh/test/ssh_compat_SUITE.erl
+++ b/lib/ssh/test/ssh_compat_SUITE.erl
@@ -92,6 +92,7 @@ end_per_suite(Config) ->
 %%%    os:cmd("docker rm $(docker ps -aq -f status=exited)"),
     %% Remove dangling images:
 %%%    os:cmd("docker rmi $(docker images -f dangling=true -q)"),
+    catch ssh:stop(),
     Config.
 
 
diff --git a/lib/ssh/test/ssh_connection_SUITE.erl b/lib/ssh/test/ssh_connection_SUITE.erl
index ba4518cfe6..77c299b3ee 100644
--- a/lib/ssh/test/ssh_connection_SUITE.erl
+++ b/lib/ssh/test/ssh_connection_SUITE.erl
@@ -85,6 +85,7 @@ init_per_suite(Config) ->
     ?CHECK_CRYPTO(Config).
 
 end_per_suite(Config) ->
+    catch ssh:stop(),
     Config.
 
 %%--------------------------------------------------------------------
@@ -800,6 +801,8 @@ stop_listener(Config) when is_list(Config) ->
 	    ssh:stop_daemon(Pid0),
 	    ssh:stop_daemon(Pid1);
 	Error ->
+	    ssh:close(ConnectionRef0),
+	    ssh:stop_daemon(Pid0),
 	    ct:fail({unexpected, Error})
     end.
 
diff --git a/lib/ssh/test/ssh_options_SUITE.erl b/lib/ssh/test/ssh_options_SUITE.erl
index bb09ca4c8b..12a85c40aa 100644
--- a/lib/ssh/test/ssh_options_SUITE.erl
+++ b/lib/ssh/test/ssh_options_SUITE.erl
@@ -208,35 +208,23 @@ end_per_group(_, Config) ->
 %%--------------------------------------------------------------------
 init_per_testcase(_TestCase, Config) ->
     ssh:start(),
-    Config.
-
-end_per_testcase(TestCase, Config) when TestCase == server_password_option;
-					TestCase == server_userpassword_option;
-					TestCase == server_pwdfun_option;
-					TestCase == server_pwdfun_4_option ;
-                                        TestCase == save_accepted_host_option ->
+    %% Create a clean user_dir
     UserDir = filename:join(proplists:get_value(priv_dir, Config), nopubkey),
     ssh_test_lib:del_dirs(UserDir),
-    end_per_testcase(Config);
-end_per_testcase(_TestCase, Config) ->
-    end_per_testcase(Config).
+    file:make_dir(UserDir),
+    [{user_dir,UserDir}|Config].
 
-end_per_testcase(_Config) ->    
-    ct:log("~p: Before ssh:stop()",[?FUNCTION_NAME]),
+end_per_testcase(_TestCase, Config) ->
     ssh:stop(),
-    ct:log("~p: After ssh:stop()",[?FUNCTION_NAME]),
     ok.
 
 %%--------------------------------------------------------------------
 %% Test Cases --------------------------------------------------------
 %%--------------------------------------------------------------------
-%%--------------------------------------------------------------------
 
 %%% validate to server that uses the 'password' option
 server_password_option(Config) when is_list(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
 					     {user_dir, UserDir},
@@ -267,12 +255,10 @@ server_password_option(Config) when is_list(Config) ->
 
 %%% validate to server that uses the 'password' option
 server_userpassword_option(Config) when is_list(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),	  
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
-					     {user_dir, PrivDir},
+					     {user_dir, UserDir},
 					     {user_passwords, [{"vego", "morot"}]}]),
 
     ConnectionRef =
@@ -302,15 +288,13 @@ server_userpassword_option(Config) when is_list(Config) ->
 %%--------------------------------------------------------------------
 %%% validate to server that uses the 'pwdfun' option
 server_pwdfun_option(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),	  
     CHKPWD = fun("foo",Pwd) -> Pwd=="bar";
 		(_,_) -> false
 	     end,
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
-					     {user_dir, PrivDir},
+					     {user_dir, UserDir},
 					     {pwdfun,CHKPWD}]),
     ConnectionRef =
 	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
@@ -340,9 +324,7 @@ server_pwdfun_option(Config) ->
 %%--------------------------------------------------------------------
 %%% validate to server that uses the 'pwdfun/4' option
 server_pwdfun_4_option(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),	  
     PWDFUN = fun("foo",Pwd,{_,_},undefined) -> Pwd=="bar";
 		("fie",Pwd,{_,_},undefined) -> {Pwd=="bar",new_state};
@@ -350,7 +332,7 @@ server_pwdfun_4_option(Config) ->
 		(_,_,_,_) -> false
 	     end,
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
-					     {user_dir, PrivDir},
+					     {user_dir, UserDir},
 					     {pwdfun,PWDFUN}]),
     ConnectionRef1 =
 	ssh_test_lib:connect(Host, Port, [{silently_accept_hosts, true},
@@ -400,9 +382,7 @@ server_pwdfun_4_option(Config) ->
     
 %%--------------------------------------------------------------------
 server_pwdfun_4_option_repeat(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),	  
     %% Test that the state works
     Parent = self(),
@@ -411,7 +391,7 @@ server_pwdfun_4_option_repeat(Config) ->
 		(_,P,_,S) -> Parent!{P,S},          {false,S+1}
 	     end,
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
-					     {user_dir, PrivDir},
+					     {user_dir, UserDir},
 					     {auth_methods,"keyboard-interactive"},
 					     {pwdfun,PWDFUN}]),
 
@@ -495,9 +475,7 @@ user_dir_option(Config) ->
 %%--------------------------------------------------------------------
 %%% validate client that uses the 'ssh_msg_debug_fun' option
 ssh_msg_debug_fun_option_client(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
@@ -535,9 +513,7 @@ ssh_msg_debug_fun_option_client(Config) ->
 
 %%--------------------------------------------------------------------
 connectfun_disconnectfun_server(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -581,9 +557,7 @@ connectfun_disconnectfun_server(Config) ->
 
 %%--------------------------------------------------------------------
 connectfun_disconnectfun_client(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -612,9 +586,7 @@ connectfun_disconnectfun_client(Config) ->
 %%--------------------------------------------------------------------
 %%% validate client that uses the 'ssh_msg_debug_fun' option
 ssh_msg_debug_fun_option_server(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -656,9 +628,7 @@ ssh_msg_debug_fun_option_server(Config) ->
 
 %%--------------------------------------------------------------------
 disconnectfun_option_server(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -691,9 +661,7 @@ disconnectfun_option_server(Config) ->
 
 %%--------------------------------------------------------------------
 disconnectfun_option_client(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -725,9 +693,7 @@ disconnectfun_option_client(Config) ->
 
 %%--------------------------------------------------------------------
 unexpectedfun_option_server(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -768,9 +734,7 @@ unexpectedfun_option_server(Config) ->
 
 %%--------------------------------------------------------------------
 unexpectedfun_option_client(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
     Parent = self(),
@@ -845,14 +809,9 @@ supported_hash(HashAlg) ->
 
 
 really_do_hostkey_fingerprint_check(Config, HashAlg) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDirServer = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDirServer),
+    UserDir = proplists:get_value(user_dir, Config),
     SysDir = proplists:get_value(data_dir, Config),
 
-    UserDirClient = 
-	ssh_test_lib:create_random_dir(Config), % Ensure no 'known_hosts' disturbs
-
     %% All host key fingerprints.  Trust that public_key has checked the ssh_hostkey_fingerprint
     %% function since that function is used by the ssh client...
     FPs0 = [case HashAlg of
@@ -878,7 +837,7 @@ really_do_hostkey_fingerprint_check(Config, HashAlg) ->
 
     %% Start daemon with the public keys that we got fingerprints from
     {Pid, Host0, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
-					     {user_dir, UserDirServer},
+					     {user_dir, UserDir},
 					     {password, "morot"}]),
     Host = ssh_test_lib:ntoa(Host0),
     FP_check_fun = fun(PeerName, FP) ->
@@ -901,7 +860,8 @@ really_do_hostkey_fingerprint_check(Config, HashAlg) ->
 				       end},
 				      {user, "foo"},
 				      {password, "morot"},
-				      {user_dir, UserDirClient},
+				      {user_dir, UserDir},
+                                      {save_accepted_host, false}, % Ensure no 'known_hosts' disturbs
 				      {user_interaction, false}]),
     ssh:stop_daemon(Pid).
 
@@ -992,9 +952,7 @@ ms_passed(T0) ->
 %%--------------------------------------------------------------------
 ssh_daemon_minimal_remote_max_packet_size_option(Config) ->
     SystemDir = proplists:get_value(data_dir, Config),
-    PrivDir = proplists:get_value(priv_dir, Config), 
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
-    file:make_dir(UserDir),
+    UserDir = proplists:get_value(user_dir, Config), 
     
     {Server, Host, Port} = ssh_test_lib:daemon([{system_dir, SystemDir},
 						{user_dir, UserDir},
@@ -1320,11 +1278,8 @@ try_to_connect(Connect, Host, Port, Pid, Tref, N) ->
 
 %%--------------------------------------------------------------------
 save_accepted_host_option(Config) ->
-    PrivDir = proplists:get_value(priv_dir, Config),
-    UserDir = filename:join(PrivDir, nopubkey), % to make sure we don't use public-key-auth
+    UserDir = proplists:get_value(user_dir, Config),
     KnownHosts = filename:join(UserDir, "known_hosts"),
-    file:make_dir(UserDir),
-    file:delete(KnownHosts),
     SysDir = proplists:get_value(data_dir, Config),	  
     {Pid, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
 					     {user_dir, UserDir},
diff --git a/lib/ssh/test/ssh_sup_SUITE.erl b/lib/ssh/test/ssh_sup_SUITE.erl
index d453a2e143..1df55834b1 100644
--- a/lib/ssh/test/ssh_sup_SUITE.erl
+++ b/lib/ssh/test/ssh_sup_SUITE.erl
@@ -201,7 +201,7 @@ killed_acceptor_restarts(Config) ->
     Port2 = ssh_test_lib:daemon_port(DaemonPid2),
     true = (Port /= Port2),
 
-    ct:pal("~s",[lists:flatten(ssh_info:string())]),
+    ct:log("~s",[lists:flatten(ssh_info:string())]),
 
     {ok,[{AccPid,ListenAddr,Port}]} = acceptor_pid(DaemonPid),
     {ok,[{AccPid2,ListenAddr,Port2}]} = acceptor_pid(DaemonPid2),
@@ -218,11 +218,14 @@ killed_acceptor_restarts(Config) ->
 
     %% Make acceptor restart:
     exit(AccPid, kill),
+    ?wait_match(undefined, process_info(AccPid)),
 
     %% Check it is a new acceptor:
-    {ok,[{AccPid1,ListenAddr,Port}]} = acceptor_pid(DaemonPid),
-    true = (AccPid /= AccPid1),
-    true = (AccPid2 /= AccPid1),
+    ?wait_match({ok,[{AccPid1,ListenAddr,Port}]}, AccPid1=/=AccPid,
+                acceptor_pid(DaemonPid),
+                AccPid1,
+                500, 30),
+    AccPid1 =/= AccPid2,
 
     %% Connect second client and check it is alive:
     {ok,C2} = ssh:connect("localhost", Port, [{silently_accept_hosts, true},
@@ -232,21 +235,21 @@ killed_acceptor_restarts(Config) ->
                                               {user_dir, UserDir}]),
     [{client_version,_}] = ssh:connection_info(C2,[client_version]),
     
-    ct:pal("~s",[lists:flatten(ssh_info:string())]),
+    ct:log("~s",[lists:flatten(ssh_info:string())]),
 
     %% Check first client is still alive:
     [{client_version,_}] = ssh:connection_info(C1,[client_version]),
     
     ok = ssh:stop_daemon(DaemonPid2),
-    timer:sleep(15000),
+    ?wait_match(undefined, process_info(DaemonPid2), 1000, 30),
     [{client_version,_}] = ssh:connection_info(C1,[client_version]),
     [{client_version,_}] = ssh:connection_info(C2,[client_version]),
     
     ok = ssh:stop_daemon(DaemonPid),
-    timer:sleep(15000),
+    ?wait_match(undefined, process_info(DaemonPid), 1000, 30),
     {error,closed} = ssh:connection_info(C1,[client_version]),
     {error,closed} = ssh:connection_info(C2,[client_version]).
-    
+
 %%-------------------------------------------------------------------------
 shell_channel_tree(Config) ->
     PrivDir = proplists:get_value(priv_dir, Config),
@@ -257,7 +260,7 @@ shell_channel_tree(Config) ->
         fun() ->
                 io:format("TimeoutShell started!~n",[]),
                 timer:sleep(5000),
-                ct:pal("~p TIMEOUT!",[self()])
+                ct:log("~p TIMEOUT!",[self()])
         end,
     {Daemon, Host, Port} = ssh_test_lib:daemon([{system_dir, SysDir},
                                                 {user_dir, UserDir},
@@ -283,14 +286,14 @@ shell_channel_tree(Config) ->
                [GroupPid]),
     {links,GroupLinks} = erlang:process_info(GroupPid, links),
     [ShellPid] = GroupLinks--[ChannelSup],
-    ct:pal("GroupPid = ~p, ShellPid = ~p",[GroupPid,ShellPid]),
+    ct:log("GroupPid = ~p, ShellPid = ~p",[GroupPid,ShellPid]),
 
     receive
         {ssh_cm,ConnectionRef, {data, ChannelId0, 0, <<"TimeoutShell started!\r\n">>}} ->
             receive
                 %%---- wait for the subsystem to terminate
                 {ssh_cm,ConnectionRef,{closed,ChannelId0}} ->
-                    ct:pal("Subsystem terminated",[]),
+                    ct:log("Subsystem terminated",[]),
                     case {chk_empty_con_daemon(Daemon),
                           process_info(GroupPid),
                           process_info(ShellPid)} of
diff --git a/lib/ssh/test/ssh_test_lib.hrl b/lib/ssh/test/ssh_test_lib.hrl
index eaf856e6e8..4b6579bd71 100644
--- a/lib/ssh/test/ssh_test_lib.hrl
+++ b/lib/ssh/test/ssh_test_lib.hrl
@@ -16,12 +16,12 @@
 %%-------------------------------------------------------------------------
 %% Help macro
 %%-------------------------------------------------------------------------
--define(wait_match(Pattern, FunctionCall, Bind, Timeout, Ntries),
+-define(wait_match(Pattern, Guard, FunctionCall, Bind, Timeout, Ntries),
 	Bind =
 	    (fun() -> 
 		     F = fun(N, F1) ->
 				 case FunctionCall of
-				     Pattern -> Bind;
+				     Pattern when Guard -> Bind;
 				     _ when N>0 ->
 					 ct:pal("Must sleep ~p ms at ~p:~p",[Timeout,?MODULE,?LINE]),
 					 timer:sleep(Timeout),
@@ -34,6 +34,9 @@
 	     end)()
        ).
 
+-define(wait_match(Pattern, FunctionCall, Bind, Timeout, Ntries),
+        ?wait_match(Pattern, true, FunctionCall, Bind, Timeout, Ntries)).
+
 -define(wait_match(Pattern, FunctionCall, Timeout, Ntries),  ?wait_match(Pattern, FunctionCall, ok, Timeout, Ntries)).
 
 -define(wait_match(Pattern, FunctionCall, Bind),  ?wait_match(Pattern, FunctionCall, Bind, 500, 10) ).