Fix error with testspec terms not being merged properly

2 files changed, 41 insertions, 48 deletions

diff --git a/lib/common_test/src/ct_testspec.erl b/lib/common_test/src/ct_testspec.erl
index b724eafe7e..c07ea323e6 100644
--- a/lib/common_test/src/ct_testspec.erl
+++ b/lib/common_test/src/ct_testspec.erl
@@ -300,8 +300,7 @@ create_testspecs(Specs,TestSpec,Relaxed,Join) ->
     %%              IncludedJoinTree, IncludedSeparateTree,
     %%              JoinSpecWithRest, RestSpecsTree}
     SpecsTree = create_spec_tree(Specs,TestSpec,Join,[]),
-    AllSpecs = create_specs(SpecsTree,TestSpec,TestSpec,Relaxed),
-    AllSpecs.
+    create_specs(SpecsTree,TestSpec,TestSpec,Relaxed).
 
 create_spec_tree([Spec|Specs],TS,JoinWithNext,Known) ->
     SpecDir = filename:dirname(filename:absname(Spec)),
@@ -364,7 +363,6 @@ create_specs({Spec,Terms,InclJoin,InclSep,JoinWithNext,NextSpec},
 		                        [SepJoinSpecs]++Separate2++
 		                        [SepJoinSpecs1]++Separate4,
 		   Ss /= []],
-
     case {JoinWithNext,JoinSpecs1} of
 	{true,_} ->
 	    {{[SpecInfo|(JoinSpecs1++JoinSpecs2)],JoinTS2},
@@ -376,7 +374,6 @@ create_specs({Spec,Terms,InclJoin,InclSep,JoinWithNext,NextSpec},
 	    {{[SpecInfo|(JoinSpecs1++JoinSpecs2)],JoinTS2},
 	     AllSeparate}
     end;
-
 create_specs([],TestSpec,_,_Relaxed) ->
     {{[],TestSpec},[]}.
 		
@@ -853,7 +850,8 @@ add_tests([{cases,Node,Dir,Suite,Cs}|Ts],Spec) ->
     Tests = Spec#testspec.tests,
     Tests1 = insert_cases(ref2node(Node,Spec#testspec.nodes),
 			  ref2dir(Dir,Spec),
-			  Suite,Cs,Tests, Spec#testspec.merge_tests),
+			  Suite,Cs,Tests,
+			  Spec#testspec.merge_tests),
     add_tests(Ts,Spec#testspec{tests=Tests1});
 
 %% --- skip_suites ---
@@ -1257,17 +1255,22 @@ insert_cases(Node,Dir,Suite,Cases,Tests,false) when is_list(Cases) ->
     append({{Node,Dir},[{Suite,Cases}]},Tests);
 insert_cases(Node,Dir,Suite,Cases,Tests,true) when is_list(Cases) ->
     {Tests1,Done} =
-	lists:foldr(fun(All={{N,D},[{all,_}]},{Replaced,_}) when N == Node,
+	lists:foldr(fun(All={{N,D},[{all,_}]},{Merged,_}) when N == Node,
 								 D == Dir ->
-			    {[All|Replaced],true};
-		       ({{N,D},Suites0},{Replaced,_}) when N == Node,
+			    {[All|Merged],true};
+		       ({{N,D},Suites0},{Merged,_}) when N == Node,
 							   D == Dir ->
 			    Suites1 = insert_cases1(Suite,Cases,Suites0),
-			    {[{{N,D},Suites1}|Replaced],true};
-		       (T,{Replaced,Match}) ->
-			    {[T|Replaced],Match}
+			    {[{{N,D},Suites1}|Merged],true};
+		       (T,{Merged,Match}) ->
+			    {[T|Merged],Match}
 		    end, {[],false}, Tests),
-    if not Done ->
+    if Tests == [] ->
+	    %% initial case with length(Cases) > 1, we need to do this
+	    %% to merge possible duplicate cases in Cases
+	    [{{Node,Dir},insert_cases1(Suite,Cases,[{Suite,[]}])}];
+	not Done ->
+	    %% no merging done, simply add these cases to Tests 
 	    Tests ++ [{{Node,Dir},[{Suite,Cases}]}];
        true ->
 	    Tests1
@@ -1312,14 +1315,14 @@ skip_groups(Node,Dir,Suite,Groups,Cases,Cmt,Tests,false) when
 skip_groups(Node,Dir,Suite,Groups,Cases,Cmt,Tests,true) when
       ((Cases == all) or is_list(Cases)) and is_list(Groups) ->
     {Tests1,Done} =
-	lists:foldr(fun({{N,D},Suites0},{Replaced,_}) when N == Node,
+	lists:foldr(fun({{N,D},Suites0},{Merged,_}) when N == Node,
 							   D == Dir ->
 			    Suites1 = skip_groups1(Suite,
 						   [{Gr,Cases} || Gr <- Groups],
 						   Cmt,Suites0),
-			    {[{{N,D},Suites1}|Replaced],true};
-		       (T,{Replaced,Match}) ->
-			    {[T|Replaced],Match}
+			    {[{{N,D},Suites1}|Merged],true};
+		       (T,{Merged,Match}) ->
+			    {[T|Merged],Match}
 		    end, {[],false}, Tests),
     if not Done ->
 	    Tests ++ [{{Node,Dir},skip_groups1(Suite,
@@ -1350,12 +1353,12 @@ skip_cases(Node,Dir,Suite,Cases,Cmt,Tests,false) when is_list(Cases) ->
     append({{Node,Dir},Suites1},Tests);
 skip_cases(Node,Dir,Suite,Cases,Cmt,Tests,true) when is_list(Cases) ->
     {Tests1,Done} =
-	lists:foldr(fun({{N,D},Suites0},{Replaced,_}) when N == Node,
+	lists:foldr(fun({{N,D},Suites0},{Merged,_}) when N == Node,
 							   D == Dir ->
 			    Suites1 = skip_cases1(Suite,Cases,Cmt,Suites0),
-			    {[{{N,D},Suites1}|Replaced],true};
-		       (T,{Replaced,Match}) ->
-			    {[T|Replaced],Match}
+			    {[{{N,D},Suites1}|Merged],true};
+		       (T,{Merged,Match}) ->
+			    {[T|Merged],Match}
 		    end, {[],false}, Tests),
     if not Done ->
 	    Tests ++ [{{Node,Dir},skip_cases1(Suite,Cases,Cmt,[])}];
diff --git a/lib/common_test/test/ct_testspec_3_SUITE.erl b/lib/common_test/test/ct_testspec_3_SUITE.erl
index d8a59b982c..5fa187e5b4 100644
--- a/lib/common_test/test/ct_testspec_3_SUITE.erl
+++ b/lib/common_test/test/ct_testspec_3_SUITE.erl
@@ -284,6 +284,24 @@ events_to_check(_, 0) ->
 events_to_check(Test, N) ->
     test_events(Test) ++ events_to_check(Test, N-1).
 
+
+%%%!
+%%%! IMPORTANT NOTE ABOUT THE TEST ORDER:
+%%%!
+%%%! When merging testspec terms, CT will group the tests by TestDir and
+%%%! Suite, before term order (in testspec). That means that if tests
+%%%! are ordered like e.g:
+%%%!   {Dir1,Suite11}, {Dir2,Suite21}, {Dir1,Suite12},
+%%%! the execution order after merge (even if no merge takes place),
+%%%! will be:
+%%%!   {Dir1,[Suite11,Suite12]}, {Dir2,Suite21}
+%%%!
+%%%! Also, tests in a tree of included testspecs are always collected
+%%%! and merged in depth-first manner, meaning even if a particular test is
+%%%! on a higher level in the tree, it may be executed later than a test on a
+%%%! lower level.
+%%%!
+
 test_events(start_separate) ->
     [{?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
@@ -552,8 +570,6 @@ test_events(incl_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 12,22,11,21?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{4,4,20}},
@@ -580,8 +596,6 @@ test_events(incl_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! WHY NOT 22,11,21,12?
-
 test_events(incl_join2) ->
     [{?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
@@ -609,8 +623,6 @@ test_events(incl_join2) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! WHY NOT 22,12,11,21?
-
 test_events(incl_both1) ->
     [{?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
@@ -709,8 +721,6 @@ test_events(incl_both2) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 11,21,12,22?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{3,2,15}},
@@ -828,8 +838,6 @@ test_events(incl_both_and_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 11,21,12,22?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{2,2,10}},
@@ -874,8 +882,6 @@ test_events(incl_both_and_join2) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 11,21,12,22?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{3,2,15}},
@@ -1078,8 +1084,6 @@ test_events(rec_incl_separate2) ->
     [{?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! OK
-
 test_events(rec_incl_join1) ->
     [
      {?eh,start_logging,{'DEF','RUNDIR'}},
@@ -1105,8 +1109,6 @@ test_events(rec_incl_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 12,22,11,21?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{5,5,25}},
@@ -1134,8 +1136,6 @@ test_events(rec_incl_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! WHY NOT 23,11,21,12,22?
-
 test_events(rec_incl_join2) ->
     [
      {?eh,start_logging,{'DEF','RUNDIR'}},
@@ -1165,8 +1165,6 @@ test_events(rec_incl_join2) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! WHY NOT 23,12,22,11,21?
-
 test_events(rec_incl_separate_join1) ->
     [
      {?eh,start_logging,{'DEF','RUNDIR'}},
@@ -1192,8 +1190,6 @@ test_events(rec_incl_separate_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 12,22,11,21?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{4,4,20}},
@@ -1217,8 +1213,6 @@ test_events(rec_incl_separate_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 22,11,21,12?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{1,1,5}},
@@ -1253,8 +1247,6 @@ test_events(rec_incl_separate_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]},
 
-%%! WHY NOT 22,11,21,12?
-
      {?eh,start_logging,{'DEF','RUNDIR'}},
      {?eh,test_start,{'DEF',{'START_TIME','LOGDIR'}}},
      {?eh,start_info,{4,4,20}},
@@ -1278,8 +1270,6 @@ test_events(rec_incl_separate_join1) ->
      {?eh,test_done,{'DEF','STOP_TIME'}},
      {?eh,stop_logging,[]}];
 
-%%! WHY NOT 12,22,11,21?
-
 test_events(rec_incl_separate_join2) ->
     [
      {?eh,start_logging,{'DEF','RUNDIR'}},