[snmp|test] Improve the test case start

The function used by "all" the agent test cases to
actually run the operations have been improved.
There was previously very little monitoring of the
result. Have added some (minor) checks, both before
trying to running the test case, and "during".
Such as, is the node we attempt to use actually alive.
Then, when we spawn the test case runner process on
the (remote) node, make it report back before trying
to run the actuall test case (so we know that the spawn
worked.
Also added a monitor of the process, so that we will
detect fatal errors.

OTP-15764

2 files changed, 119 insertions, 66 deletions

diff --git a/lib/snmp/test/snmp_agent_test_lib.erl b/lib/snmp/test/snmp_agent_test_lib.erl
index 6defdadb5a..c1fcaf27d6 100644
--- a/lib/snmp/test/snmp_agent_test_lib.erl
+++ b/lib/snmp/test/snmp_agent_test_lib.erl
@@ -66,7 +66,7 @@
 	]).
 
 %% Internal exports
--export([wait/5, run/4]).
+-export([tc_wait/5, tc_run/4]).
 
 -include_lib("kernel/include/file.hrl").
 -include_lib("common_test/include/ct.hrl").
@@ -277,35 +277,80 @@ init_case(Config) when is_list(Config) ->
 %%%--------------------------------------------------
 
 try_test(Mod, Func) ->
-    call(get(mgr_node), ?MODULE, run, [Mod, Func, [], []]).
+    tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, [], []]).
 
 try_test(Mod, Func, A) ->
-    call(get(mgr_node), ?MODULE, run, [Mod, Func, A, []]).
+    tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, []]).
 
 try_test(Mod, Func, A, Opts) ->
-    call(get(mgr_node), ?MODULE, run, [Mod, Func, A, Opts]).
-
-call(N,M,F,A) ->
-    ?DBG("call -> entry with~n"
-	   "    N:     ~p~n"
-	   "    M:     ~p~n"
-	   "    F:     ~p~n"
-	   "    A:     ~p~n"
-	   "  when~n"
-	   "    get(): ~p",
-	   [N,M,F,A,get()]),
-    spawn(N, ?MODULE, wait, [self(),get(),M,F,A]),
+    tc_try(get(mgr_node), ?MODULE, tc_run, [Mod, Func, A, Opts]).
+
+%% We spawn a test case runner process on the manager node.
+%% The assumption is that the manager shall do something, but
+%% not all test cases have the manager perform actions.
+%% In some cases we make a rpc call back to the agent node directly
+%% and call something in the agent... (for example the info_test
+%% test case).
+tc_try(N, M, F, A) ->
+    ?PRINT2("tc_try -> entry with"
+            "~n      N:     ~p"
+            "~n      M:     ~p"
+            "~n      F:     ~p"
+            "~n      A:     ~p"
+            "~n   when"
+            "~n      get(): ~p"
+            "~n", [N,
+                   M, F, A,
+                   get()]),
+    case net_adm:ping(N) of
+        pong ->
+            ?PRINT2("tc_try -> ~p still running~n", [N]),
+            Runner = spawn(N, ?MODULE, tc_wait, [self(), get(), M, F, A]),
+            MRef   = erlang:monitor(process, Runner),
+            await_tc_runner_started(Runner, MRef),
+            await_tc_runner_done(Runner, MRef);
+        pang ->
+            ?EPRINT2("tc_try -> ~p *not* running~n", [N]),
+            exit({node_not_running, N})
+    end.
+
+await_tc_runner_started(Runner, MRef) ->
     receive
-	{done, {'EXIT', Rn}, Loc} ->
-	    ?DBG("call -> done with exit: "
-		 "~n   Rn:  ~p"
-		 "~n   Loc: ~p", [Rn, Loc]),
+        {'DOWN', MRef, process, Runner, Reason} ->
+            ?EPRINT2("TC runner start failed: "
+                     "~n   ~p~n", [Reason]),
+            exit({tx_runner_start_failed, Reason});
+        {tc_runner_started, Runner} ->
+            ?PRINT2("TC runner start acknowledged~n"),
+            ok
+    after 10000 ->
+            erlang:demonitor(MRef, [flush]),
+            RunnerInfo = process_info(Runner),
+            ?EPRINT2("TC runner start timeout: "
+                     "~n   ~p", [RunnerInfo]),
+            %% If we don't get a start ack within 10 seconds, we are f*ed
+            exit(Runner, kill),
+            exit({tc_runner_start, timeout, RunnerInfo})
+    end.
+
+await_tc_runner_done(Runner, MRef) ->
+    receive
+        {'DOWN', MRef, process, Runner, Reason} ->
+            ?EPRINT2("TC runner failed: "
+                     "~n   ~p~n", [Reason]),
+            exit({tx_runner_failed, Reason});
+	{tc_runner_done, Runner, {'EXIT', Rn}, Loc} ->
+	    ?PRINT2("call -> done with exit: "
+                    "~n   Rn:  ~p"
+                    "~n   Loc: ~p"
+                    "~n", [Rn, Loc]),
 	    put(test_server_loc, Loc),
 	    exit(Rn);
-	{done, Ret, _Zed} -> 
+	{tc_runner_done, Runner, Ret, _Zed} -> 
 	    ?DBG("call -> done:"
 		 "~n   Ret: ~p"
 		 "~n   Zed: ~p", [Ret, _Zed]),
+            erlang:demonitor(MRef, [flush]),
 	    case Ret of
 		{error, Reason} ->
 		    exit(Reason);
@@ -313,50 +358,56 @@ call(N,M,F,A) ->
 		    OK
 	    end
     end.
-
-wait(From, Env, M, F, A) ->
-    ?DBG("wait -> entry with"
-	 "~n   From: ~p"
-	 "~n   Env:  ~p"
-	 "~n   M:    ~p"
-	 "~n   F:    ~p"
-	 "~n   A:    ~p", [From, Env, M, F, A]),
+    
+    
+tc_wait(From, Env, M, F, A) ->
+    ?PRINT2("tc_wait -> entry with"
+            "~n   From: ~p"
+            "~n   Env:  ~p"
+            "~n   M:    ~p"
+            "~n   F:    ~p"
+            "~n   A:    ~p", [From, Env, M, F, A]),
+    From ! {tc_runner_started, self()},
     lists:foreach(fun({K,V}) -> put(K,V) end, Env),
-    Rn = (catch apply(M, F, A)),
-    ?DBG("wait -> Rn: ~n~p", [Rn]),
-    From ! {done, Rn, get(test_server_loc)},
-    exit(Rn).
-
-run(Mod, Func, Args, Opts) ->
-    ?DBG("run -> entry with"
-	 "~n   Mod:  ~p"
-	 "~n   Func: ~p"
-	 "~n   Args: ~p"
-	 "~n   Opts: ~p", [Mod, Func, Args, Opts]),
-    M = get(mib_dir),
-    Dir = get(mgr_dir),
-    User = snmp_misc:get_option(user, Opts, "all-rights"),
-    SecLevel = snmp_misc:get_option(sec_level, Opts, noAuthNoPriv),
-    EngineID = snmp_misc:get_option(engine_id, Opts, "agentEngine"),
+    ?PRINT2("tc_wait -> env set - now run tc~n"),
+    Res = (catch apply(M, F, A)),
+    ?PRINT2("tc_wait -> tc run done: "
+            "~n   ~p"
+            "~n", [Res]),
+    From ! {tc_runner_done, self(), Res, get(test_server_loc)},
+    exit(Res).
+
+tc_run(Mod, Func, Args, Opts) ->
+    ?PRINT2("tc_run -> entry with"
+            "~n   Mod:  ~p"
+            "~n   Func: ~p"
+            "~n   Args: ~p"
+            "~n   Opts: ~p"
+            "~n", [Mod, Func, Args, Opts]),
+    (catch snmp_test_mgr:stop()), % If we had a running mgr from a failed case
+    M           = get(mib_dir),
+    Dir         = get(mgr_dir),
+    User        = snmp_misc:get_option(user, Opts, "all-rights"),
+    SecLevel    = snmp_misc:get_option(sec_level, Opts, noAuthNoPriv),
+    EngineID    = snmp_misc:get_option(engine_id, Opts, "agentEngine"),
     CtxEngineID = snmp_misc:get_option(context_engine_id, Opts, EngineID),
-    Community = snmp_misc:get_option(community, Opts, "all-rights"),
-    ?DBG("run -> start crypto app",[]),
-    _CryptoRes = ?CRYPTO_START(),
-    ?DBG("run -> Crypto: ~p", [_CryptoRes]),
-    catch snmp_test_mgr:stop(), % If we had a running mgr from a failed case
-    StdM = join(code:priv_dir(snmp), "mibs") ++ "/",
-    Vsn = get(vsn), 
-    ?DBG("run -> config:"
-	   "~n   M:           ~p"
-	   "~n   Vsn:         ~p"
-	   "~n   Dir:         ~p"
-	   "~n   User:        ~p"
-	   "~n   SecLevel:    ~p"
-	   "~n   EngineID:    ~p"
-	   "~n   CtxEngineID: ~p"
-	   "~n   Community:   ~p"
-	   "~n   StdM:        ~p",
-	   [M,Vsn,Dir,User,SecLevel,EngineID,CtxEngineID,Community,StdM]),
+    Community   = snmp_misc:get_option(community, Opts, "all-rights"),
+    ?DBG("tc_run -> start crypto app",[]),
+    _CryptoRes  = ?CRYPTO_START(),
+    ?DBG("tc_run -> Crypto: ~p", [_CryptoRes]),
+    StdM        = join(code:priv_dir(snmp), "mibs") ++ "/",
+    Vsn         = get(vsn), 
+    ?PRINT2("tc_run -> config:"
+            "~n   M:           ~p"
+            "~n   Vsn:         ~p"
+            "~n   Dir:         ~p"
+            "~n   User:        ~p"
+            "~n   SecLevel:    ~p"
+            "~n   EngineID:    ~p"
+            "~n   CtxEngineID: ~p"
+            "~n   Community:   ~p"
+            "~n   StdM:        ~p"
+            "~n", [M,Vsn,Dir,User,SecLevel,EngineID,CtxEngineID,Community,StdM]),
     case snmp_test_mgr:start([%% {agent, snmp_test_lib:hostname()},
 			      {packet_server_debug, true},
 			      {debug,               true},
@@ -377,23 +428,23 @@ run(Mod, Func, Args, Opts) ->
 	{ok, _Pid} ->
 	    case (catch apply(Mod, Func, Args)) of
 		{'EXIT', Reason} ->
-		    catch snmp_test_mgr:stop(),
+		    (catch snmp_test_mgr:stop()),
 		    ?FAIL({apply_failed, {Mod, Func, Args}, Reason});
 		Res ->
-		    catch snmp_test_mgr:stop(),
+		    (catch snmp_test_mgr:stop()),
 		    Res
 	    end;
 
 	{error, Reason} ->
 	    ?EPRINT2("Failed starting (test) manager: "
                      "~n   ~p", [Reason]),
-	    catch snmp_test_mgr:stop(),
+	    (catch snmp_test_mgr:stop()),
 	    ?line ?FAIL({mgr_start_error, Reason});
 
 	Err ->
 	    ?EPRINT2("Failed starting (test) manager: "
                      "~n   ~p", [Err]),
-	    catch snmp_test_mgr:stop(),
+	    (catch snmp_test_mgr:stop()),
 	    ?line ?FAIL({mgr_start_failure, Err})
     end.
 
diff --git a/lib/snmp/test/snmp_test_lib.hrl b/lib/snmp/test/snmp_test_lib.hrl
index 335f3fff3c..dafa4cbf19 100644
--- a/lib/snmp/test/snmp_test_lib.hrl
+++ b/lib/snmp/test/snmp_test_lib.hrl
@@ -150,8 +150,10 @@
 	snmp_test_lib:print(P, ?MODULE, ?LINE, F, A)).
 
 -define(PRINT1(F, A),  snmp_test_lib:print1(F, A)).
+-define(PRINT1(F),     ?PRINT1(F, [])).
 -define(EPRINT1(F, A), ?PRINT1("<ERROR> " ++ F, A)).
 
 -define(PRINT2(F, A),  snmp_test_lib:print2(F, A)).
+-define(PRINT2(F),     ?PRINT2(F, [])).
 -define(EPRINT2(F, A), ?PRINT2("<ERROR> " ++ F, A)).