detect and report bad return values from generators and instantiators

3 files changed, 67 insertions, 15 deletions

diff --git a/lib/eunit/src/eunit_data.erl b/lib/eunit/src/eunit_data.erl
index 392d378a0e..da66864439 100644
--- a/lib/eunit/src/eunit_data.erl
+++ b/lib/eunit/src/eunit_data.erl
@@ -156,7 +156,8 @@ iter_prev(#iter{prev = [T | Ts]} = I) ->
 %% @spec (tests()) -> none | {testItem(), tests()}
 %% @type testItem() = #test{} | #group{}
 %% @throws {bad_test, term()}
-%%       | {generator_failed, exception()}
+%%       | {generator_failed, {{M::atom(),F::atom(),A::integer()},
+%%                             exception()}}
 %%       | {no_such_function, eunit_lib:mfa()}
 %%       | {module_not_found, moduleName()}
 %%       | {application_not_found, appName()}
@@ -221,17 +222,27 @@ parse({foreachx, P, S1, C1, Ps} = T)
 	[] ->
 	    {data, []}
     end;
-parse({generator, F} = T) when is_function(F) ->
+parse({generator, F}) when is_function(F) ->
+    {module, M} = erlang:fun_info(F, module),
+    {name, N} = erlang:fun_info(F, name),
+    {arity, A} = erlang:fun_info(F, arity),
+    parse({generator, F, {M,N,A}});
+parse({generator, F, {M,N,A}} = T)
+  when is_function(F), is_atom(M), is_atom(N), is_integer(A) ->
     check_arity(F, 0, T),
     %% use run_testfun/1 to handle wrapper exceptions
     case eunit_test:run_testfun(F) of
 	{ok, T1} ->
+            case eunit_lib:is_not_test(T1) of
+                true -> throw({bad_generator, {{M,N,A}, T1}});
+                false -> ok
+            end,
 	    {data, T1};
 	{error, {Class, Reason, Trace}} ->
-	    throw({generator_failed, {Class, Reason, Trace}})
+	    throw({generator_failed, {{M,N,A}, {Class, Reason, Trace}}})
     end;
 parse({generator, M, F}) when is_atom(M), is_atom(F) ->
-    parse({generator, eunit_test:function_wrapper(M, F)});
+    parse({generator, eunit_test:mf_wrapper(M, F), {M,F,0}});
 parse({inorder, T}) ->
     group(#group{tests = T, order = inorder});
 parse({inparallel, T}) ->
@@ -422,7 +433,7 @@ parse_function(F) when is_function(F) ->
     check_arity(F, 0, F),
     #test{f = F, location = eunit_lib:fun_parent(F)};
 parse_function({M,F}) when is_atom(M), is_atom(F) ->
-    #test{f = eunit_test:function_wrapper(M, F), location = {M, F, 0}};
+    #test{f = eunit_test:mf_wrapper(M, F), location = {M, F, 0}};
 parse_function(F) ->
     bad_test(F).
 
diff --git a/lib/eunit/src/eunit_lib.erl b/lib/eunit/src/eunit_lib.erl
index 5d7025f038..f0fd26eb5d 100644
--- a/lib/eunit/src/eunit_lib.erl
+++ b/lib/eunit/src/eunit_lib.erl
@@ -30,7 +30,8 @@
 -export([dlist_next/1, uniq/1, fun_parent/1, is_string/1, command/1,
 	 command/2, command/3, trie_new/0, trie_store/2, trie_match/2,
 	 split_node/1, consult_file/1, list_dir/1, format_exit_term/1,
-	 format_exception/1, format_exception/2, format_error/1]).
+	 format_exception/1, format_exception/2, format_error/1,
+         is_not_test/1]).
 
 
 %% Type definitions for describing exceptions
@@ -158,9 +159,13 @@ is_op(_M, _F, _A) ->
 
 format_error({bad_test, Term}) ->
     error_msg("bad test descriptor", "~P", [Term, 15]);
-format_error({generator_failed, Exception}) ->
-    error_msg("test generator failed", "~s",
-	      [format_exception(Exception)]);
+format_error({bad_generator, {{M,F,A}, Term}}) ->
+    error_msg(io_lib:format("result from generator ~w:~w/~w is not a test",
+                            [M,F,A]),
+              "~P", [Term, 15]);
+format_error({generator_failed, {{M,F,A}, Exception}}) ->
+    error_msg(io_lib:format("test generator ~w:~w/~w failed",[M,F,A]),
+              "~s", [format_exception(Exception)]);
 format_error({no_such_function, {M,F,A}})
   when is_atom(M), is_atom(F), is_integer(A) ->
     error_msg(io_lib:format("no such function: ~w:~w/~w", [M,F,A]),
@@ -177,6 +182,10 @@ format_error({setup_failed, Exception}) ->
 format_error({cleanup_failed, Exception}) ->
     error_msg("context cleanup failed", "~s",
 	      [format_exception(Exception)]);
+format_error({{bad_instantiator, {{M,F,A}, Term}}, _DummyException}) ->
+    error_msg(io_lib:format("result from instantiator ~w:~w/~w is not a test",
+                            [M,F,A]),
+              "~P", [Term, 15]);
 format_error({instantiation_failed, Exception}) ->
     error_msg("instantiation of subtests failed", "~s",
 	      [format_exception(Exception)]).
@@ -202,6 +211,28 @@ format_exception_test_() ->
 -endif.
 
 %% ---------------------------------------------------------------------
+%% detect common return values that are definitely not tests
+
+is_not_test(T) ->
+    case T of
+        ok -> true;
+        error -> true;
+        true -> true;
+        false -> true;
+        undefined -> true;
+        {ok, _} -> true;
+        {error, _} -> true;
+        {'EXIT', _} -> true;
+        N when is_number(N) -> true;
+        [N|_] when is_number(N) -> true;
+        X when is_binary(X) -> true;
+        X when is_pid(X) -> true;
+        X when is_port(X) -> true;
+        X when is_reference(X) -> true;
+        _ -> false
+    end.
+
+%% ---------------------------------------------------------------------
 %% Deep list iterator; accepts improper lists/sublists, and also accepts
 %% non-lists on the top level. Nonempty strings (not deep strings) are
 %% recognized as separate elements, even on the top level. (It is not
diff --git a/lib/eunit/src/eunit_test.erl b/lib/eunit/src/eunit_test.erl
index 3457b5ee21..9cf40a738d 100644
--- a/lib/eunit/src/eunit_test.erl
+++ b/lib/eunit/src/eunit_test.erl
@@ -21,8 +21,7 @@
 
 -module(eunit_test).
 
--export([run_testfun/1, function_wrapper/2, enter_context/4,
-	 multi_setup/1]).
+-export([run_testfun/1, mf_wrapper/2, enter_context/4, multi_setup/1]).
 
 
 -include("eunit.hrl").
@@ -262,7 +261,7 @@ macro_test_() ->
 %% @type wrapperError() = {no_such_function, mfa()}
 %%                      | {module_not_found, moduleName()}
 
-function_wrapper(M, F) ->
+mf_wrapper(M, F) ->
     fun () ->
  	    try M:F()
  	    catch
@@ -293,12 +292,12 @@ fail(Term) ->
 wrapper_test_() ->
     {"error handling in function wrapper",
      [?_assertException(throw, {module_not_found, eunit_nonexisting},
-  			run_testfun(function_wrapper(eunit_nonexisting,test))),
+  			run_testfun(mf_wrapper(eunit_nonexisting,test))),
       ?_assertException(throw,
   			{no_such_function, {?MODULE,nonexisting_test,0}},
-  			run_testfun(function_wrapper(?MODULE,nonexisting_test))),
+  			run_testfun(mf_wrapper(?MODULE,nonexisting_test))),
       ?_test({error, {error, undef, _T}}
-  	     = run_testfun(function_wrapper(?MODULE,wrapper_test_exported_)))
+  	     = run_testfun(mf_wrapper(?MODULE,wrapper_test_exported_)))
      ]}.
 
 %% this must be exported (done automatically by the autoexport transform)
@@ -323,6 +322,17 @@ enter_context(Setup, Cleanup, Instantiate, Callback) ->
 	R ->
 	    try Instantiate(R) of
 		T ->
+                    case eunit_lib:is_not_test(T) of
+                        true ->
+                            catch throw(error),  % generate a stack trace
+                            {module,M} = erlang:fun_info(Instantiate, module),
+                            {name,N} = erlang:fun_info(Instantiate, name),
+                            {arity,A} = erlang:fun_info(Instantiate, arity),
+                            context_error({bad_instantiator, {{M,N,A},T}},
+                                          error, badarg);
+                        false ->
+                            ok
+                    end,
 		    try Callback(T)  %% call back to client code
 		    after
 			%% Always run cleanup; client may be an idiot