From 85e00f2a434d3295d7a683838cda002403f5af17 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Mon, 28 Nov 2016 11:45:21 +0100 Subject: Improve merge of command line options and config options Bug: option 'spec' is not specifically handled when merging options from the command line with options from rebar.config. Due to this, if the config specifies a 'spec', then this will take precedence over any 'dir' and/or 'suite' on the command line. This commit takes special care of all options that can be used to select tests - meaning that if any of the options 'spec', 'dir', 'suite', 'group' or 'case' are specified on the command line, then all 'spec', 'dir', 'suite', 'group' and 'case' options in rebar.config will be ignored. --- src/rebar_prv_common_test.erl | 77 +++++++++++++++++++++++++++++-------------- 1 file changed, 52 insertions(+), 25 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index be31e8c..5fa7ae4 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -227,20 +227,42 @@ select_tests(State, ProjectApps, CmdOpts, CfgOpts) -> rebar_utils:reread_config(Configs), code:set_path(OldPath), - Merged = lists:ukeymerge(1, - lists:ukeysort(1, CmdOpts), - lists:ukeysort(1, CfgOpts)), - %% make sure `dir` and/or `suite` from command line go in as - %% a pair overriding both `dir` and `suite` from config if - %% they exist - Opts = case {proplists:get_value(suite, CmdOpts), proplists:get_value(dir, CmdOpts)} of - {undefined, undefined} -> Merged; - {_Suite, undefined} -> lists:keydelete(dir, 1, Merged); - {undefined, _Dir} -> lists:keydelete(suite, 1, Merged); - {_Suite, _Dir} -> Merged - end, + Opts = merge_opts(CmdOpts,CfgOpts), discover_tests(State, ProjectApps, Opts). +%% Merge the option lists from command line and rebar.config: +%% +%% - Options set on the command line will replace the same options if +%% set in rebar.config. +%% +%% - Special care is taken with options that select which tests to +%% run - ANY such option on the command line will replace ALL such +%% options in the config. +%% +%% Note that if 'spec' is given, common_test will ignore all 'dir', +%% 'suite', 'group' and 'case', so there is no need to explicitly +%% remove any options from the command line. +%% +%% All faulty combinations of options are also handled by +%% common_test and are not taken into account here. +merge_opts(CmdOpts0, CfgOpts0) -> + TestSelectOpts = [spec,dir,suite,group,testcase], + CmdOpts = lists:ukeysort(1, CmdOpts0), + CfgOpts1 = lists:ukeysort(1, CfgOpts0), + CfgOpts = case is_any_defined(TestSelectOpts,CmdOpts) of + false -> + CfgOpts1; + true -> + [Opt || Opt={K,_} <- CfgOpts1, + not lists:member(K,TestSelectOpts)] + end, + lists:ukeymerge(1, CmdOpts, CfgOpts). + +is_any_defined([Key|Keys],Opts) -> + proplists:is_defined(Key,Opts) orelse is_any_defined(Keys,Opts); +is_any_defined([],_Opts) -> + false. + sys_config_list(CmdOpts, CfgOpts) -> CmdSysConfigs = split_string(proplists:get_value(sys_config, CmdOpts, "")), case proplists:get_value(sys_config, CfgOpts, []) of @@ -253,11 +275,10 @@ sys_config_list(CmdOpts, CfgOpts) -> end. discover_tests(State, ProjectApps, Opts) -> - case {proplists:get_value(suite, Opts), proplists:get_value(dir, Opts)} of - %% no dirs or suites defined, try using `$APP/test` and `$ROOT/test` - %% as suites - {undefined, undefined} -> {ok, [default_tests(State, ProjectApps)|Opts]}; - {_, _} -> {ok, Opts} + case is_any_defined([spec,dir,suite],Opts) of + %% no tests defined, try using `$APP/test` and `$ROOT/test` as dirs + false -> {ok, [default_tests(State, ProjectApps)|Opts]}; + true -> {ok, Opts} end. default_tests(State, ProjectApps) -> @@ -397,14 +418,20 @@ readable(State) -> end. test_dirs(State, Apps, Opts) -> - case {proplists:get_value(suite, Opts), proplists:get_value(dir, Opts)} of - {Suites, undefined} -> set_compile_dirs(State, Apps, {suite, Suites}); - {undefined, Dirs} -> set_compile_dirs(State, Apps, {dir, Dirs}); - {Suites, Dir} when is_integer(hd(Dir)) -> - set_compile_dirs(State, Apps, join(Suites, Dir)); - {Suites, [Dir]} when is_integer(hd(Dir)) -> - set_compile_dirs(State, Apps, join(Suites, Dir)); - {_Suites, _Dirs} -> {error, "Only a single directory may be specified when specifying suites"} + case proplists:get_value(spec, Opts) of + undefined -> + case {proplists:get_value(suite, Opts), proplists:get_value(dir, Opts)} of + {Suites, undefined} -> set_compile_dirs(State, Apps, {suite, Suites}); + {undefined, Dirs} -> set_compile_dirs(State, Apps, {dir, Dirs}); + {Suites, Dir} when is_integer(hd(Dir)) -> + set_compile_dirs(State, Apps, join(Suites, Dir)); + {Suites, [Dir]} when is_integer(hd(Dir)) -> + set_compile_dirs(State, Apps, join(Suites, Dir)); + {_Suites, _Dirs} -> {error, "Only a single directory may be specified when specifying suites"} + end; + _Specs -> + %% Currently not adding any directories from the spec files. + {ok, rebar_state:project_apps(State, Apps)} end. join(Suite, Dir) when is_integer(hd(Suite)) -> -- cgit v1.1 From a0e7ff2eb95a8093f666d310bd5df6395a243bd8 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Thu, 8 Dec 2016 15:46:35 +0100 Subject: Add directory of testspec as extra_src_dir This is necessary in order to automatically get the testspec included as an artifact (i.e. copied to the _build dir) in the case when it is stored in another directory than 'test'. --- src/rebar_prv_common_test.erl | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 5fa7ae4..17358a5 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -429,9 +429,9 @@ test_dirs(State, Apps, Opts) -> set_compile_dirs(State, Apps, join(Suites, Dir)); {_Suites, _Dirs} -> {error, "Only a single directory may be specified when specifying suites"} end; - _Specs -> - %% Currently not adding any directories from the spec files. - {ok, rebar_state:project_apps(State, Apps)} + Specs -> + set_compile_dirs(State, Apps, {spec, Specs}) + end. join(Suite, Dir) when is_integer(hd(Suite)) -> @@ -451,15 +451,15 @@ set_compile_dirs(State, Apps, {dir, Dirs}) -> F = fun(Dir, {S, A}) -> maybe_inject_test_dir(S, [], A, Dir) end, {NewState, NewApps} = lists:foldl(F, {State, Apps}, Dirs), {ok, rebar_state:project_apps(NewState, NewApps)}; -set_compile_dirs(State, Apps, {suite, Suites}) -> - %% suites with dir component - Dirs = find_suite_dirs(Suites), +set_compile_dirs(State, Apps, {Type, Files}) when Type==spec; Type==suite -> + %% specs or suites with dir component + Dirs = find_file_dirs(Files), F = fun(Dir, {S, A}) -> maybe_inject_test_dir(S, [], A, Dir) end, {NewState, NewApps} = lists:foldl(F, {State, Apps}, Dirs), {ok, rebar_state:project_apps(NewState, NewApps)}. -find_suite_dirs(Suites) -> - AllDirs = lists:map(fun(S) -> filename:dirname(filename:absname(S)) end, Suites), +find_file_dirs(Files) -> + AllDirs = lists:map(fun(F) -> filename:dirname(filename:absname(F)) end, Files), %% eliminate duplicates lists:usort(AllDirs). -- cgit v1.1 From e427a835301c41a1403cf6b9e6c363ceae3e781c Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Fri, 9 Dec 2016 14:33:43 +0100 Subject: Translate path to testspec This is a bugfix. It makes sure that the given path to a testspec is translated so common_test will pick the spec from the _build directory, and not from the source tree. --- src/rebar_prv_common_test.erl | 61 ++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 33 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 17358a5..cb33492 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -513,44 +513,39 @@ inject_test_dir(Opts, Dir) -> rebar_opts:set(Opts, extra_src_dirs, ExtraSrcDirs ++ [Dir]). translate_paths(State, Opts) -> - case {proplists:get_value(suite, Opts), proplists:get_value(dir, Opts)} of - {_Suites, undefined} -> translate_suites(State, Opts, []); - {undefined, _Dirs} -> translate_dirs(State, Opts, []); - %% both dirs and suites are defined, only translate dir paths - _ -> translate_dirs(State, Opts, []) + case proplists:get_value(spec, Opts) of + undefined -> + case {proplists:get_value(suite, Opts), proplists:get_value(dir, Opts)} of + {_Suites, undefined} -> translate_paths(State, suite, Opts, []); + {undefined, _Dirs} -> translate_paths(State, dir, Opts, []); + %% both dirs and suites are defined, only translate dir paths + _ -> translate_paths(State, dir, Opts, []) + end; + _Specs -> + translate_paths(State, spec, Opts, []) end. -translate_dirs(_State, [], Acc) -> lists:reverse(Acc); -translate_dirs(State, [{dir, Dir}|Rest], Acc) when is_integer(hd(Dir)) -> - %% single dir - Apps = rebar_state:project_apps(State), - translate_dirs(State, Rest, [{dir, translate(State, Apps, Dir)}|Acc]); -translate_dirs(State, [{dir, Dirs}|Rest], Acc) -> - %% multiple dirs - Apps = rebar_state:project_apps(State), - NewDirs = {dir, lists:map(fun(Dir) -> translate(State, Apps, Dir) end, Dirs)}, - translate_dirs(State, Rest, [NewDirs|Acc]); -translate_dirs(State, [Test|Rest], Acc) -> - translate_dirs(State, Rest, [Test|Acc]). - -translate_suites(_State, [], Acc) -> lists:reverse(Acc); -translate_suites(State, [{suite, Suite}|Rest], Acc) when is_integer(hd(Suite)) -> - %% single suite +translate_paths(_State, _Type, [], Acc) -> lists:reverse(Acc); +translate_paths(State, Type, [{Type, Val}|Rest], Acc) when is_integer(hd(Val)) -> + %% single file or dir + translate_paths(State, Type, [{Type, [Val]}|Rest], Acc); +translate_paths(State, dir, [{dir, Dirs}|Rest], Acc) -> Apps = rebar_state:project_apps(State), - translate_suites(State, Rest, [{suite, translate_suite(State, Apps, Suite)}|Acc]); -translate_suites(State, [{suite, Suites}|Rest], Acc) -> - %% multiple suites + New = {dir, lists:map(fun(Dir) -> translate(State, Apps, Dir) end, Dirs)}, + translate_paths(State, dir, Rest, [New|Acc]); +translate_paths(State, Type, [{Type, Files}|Rest], Acc) -> + %% Type = suites | specs Apps = rebar_state:project_apps(State), - NewSuites = {suite, lists:map(fun(Suite) -> translate_suite(State, Apps, Suite) end, Suites)}, - translate_suites(State, Rest, [NewSuites|Acc]); -translate_suites(State, [Test|Rest], Acc) -> - translate_suites(State, Rest, [Test|Acc]). - -translate_suite(State, Apps, Suite) -> - Dirname = filename:dirname(Suite), - Basename = filename:basename(Suite), + New = {Type, lists:map(fun(File) -> translate_file(State, Apps, File) end, Files)}, + translate_paths(State, Type, Rest, [New|Acc]); +translate_paths(State, Type, [Test|Rest], Acc) -> + translate_paths(State, Type, Rest, [Test|Acc]). + +translate_file(State, Apps, File) -> + Dirname = filename:dirname(File), + Basename = filename:basename(File), case Dirname of - "." -> Suite; + "." -> File; _ -> filename:join([translate(State, Apps, Dirname), Basename]) end. -- cgit v1.1 From a18340c6eed6e62237c4ec63f15daacfe86dc86c Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Tue, 13 Dec 2016 12:51:01 +0100 Subject: Allow using relative path to suite in project root --- src/rebar_prv_common_test.erl | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index cb33492..9db8106 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -529,26 +529,13 @@ translate_paths(_State, _Type, [], Acc) -> lists:reverse(Acc); translate_paths(State, Type, [{Type, Val}|Rest], Acc) when is_integer(hd(Val)) -> %% single file or dir translate_paths(State, Type, [{Type, [Val]}|Rest], Acc); -translate_paths(State, dir, [{dir, Dirs}|Rest], Acc) -> - Apps = rebar_state:project_apps(State), - New = {dir, lists:map(fun(Dir) -> translate(State, Apps, Dir) end, Dirs)}, - translate_paths(State, dir, Rest, [New|Acc]); translate_paths(State, Type, [{Type, Files}|Rest], Acc) -> - %% Type = suites | specs Apps = rebar_state:project_apps(State), - New = {Type, lists:map(fun(File) -> translate_file(State, Apps, File) end, Files)}, + New = {Type, lists:map(fun(File) -> translate(State, Apps, File) end, Files)}, translate_paths(State, Type, Rest, [New|Acc]); translate_paths(State, Type, [Test|Rest], Acc) -> translate_paths(State, Type, Rest, [Test|Acc]). -translate_file(State, Apps, File) -> - Dirname = filename:dirname(File), - Basename = filename:basename(File), - case Dirname of - "." -> File; - _ -> filename:join([translate(State, Apps, Dirname), Basename]) - end. - translate(State, [App|Rest], Path) -> case rebar_file_utils:path_from_ancestor(Path, rebar_app_info:dir(App)) of {ok, P} -> filename:join([rebar_app_info:out_dir(App), P]); -- cgit v1.1 From 4e52ce58e505ba57f2188e34a002d70de1bd6ff5 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Fri, 16 Dec 2016 10:34:36 +0100 Subject: Add all dirs from test spec Parse given test specs and add all spec- and suite directories as extra_src_dirs in order to ensure that all these directories are copied to the _build area and the suites are compiled. Specs located in the project- or app root are explicitly copied to the _build area in order to avoid recursive copying of the complete directory tree. --- src/rebar_prv_common_test.erl | 81 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 69 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 9db8106..4a9ba25 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -429,9 +429,18 @@ test_dirs(State, Apps, Opts) -> set_compile_dirs(State, Apps, join(Suites, Dir)); {_Suites, _Dirs} -> {error, "Only a single directory may be specified when specifying suites"} end; - Specs -> - set_compile_dirs(State, Apps, {spec, Specs}) - + Specs0 -> + case get_dirs_from_specs(Specs0) of + {ok,{Specs,SuiteDirs}} -> + {State1,Apps1} = set_compile_dirs1(State, Apps, + {dir, SuiteDirs}), + {State2,Apps2} = set_compile_dirs1(State1, Apps1, + {spec, Specs}), + [maybe_copy_spec(State2,Apps2,S) || S <- Specs], + {ok, rebar_state:project_apps(State2, Apps2)}; + Error -> + Error + end end. join(Suite, Dir) when is_integer(hd(Suite)) -> @@ -439,24 +448,25 @@ join(Suite, Dir) when is_integer(hd(Suite)) -> join(Suites, Dir) -> {suite, lists:map(fun(S) -> filename:join([Dir, S]) end, Suites)}. -set_compile_dirs(State, Apps, {dir, Dir}) when is_integer(hd(Dir)) -> +set_compile_dirs(State, Apps, What) -> + {NewState,NewApps} = set_compile_dirs1(State, Apps, What), + {ok, rebar_state:project_apps(NewState, NewApps)}. + +set_compile_dirs1(State, Apps, {dir, Dir}) when is_integer(hd(Dir)) -> %% single directory %% insert `Dir` into an app if relative, or the base state if not %% app relative but relative to the root or not at all if outside %% project scope - {NewState, NewApps} = maybe_inject_test_dir(State, [], Apps, Dir), - {ok, rebar_state:project_apps(NewState, NewApps)}; -set_compile_dirs(State, Apps, {dir, Dirs}) -> + maybe_inject_test_dir(State, [], Apps, Dir); +set_compile_dirs1(State, Apps, {dir, Dirs}) -> %% multiple directories F = fun(Dir, {S, A}) -> maybe_inject_test_dir(S, [], A, Dir) end, - {NewState, NewApps} = lists:foldl(F, {State, Apps}, Dirs), - {ok, rebar_state:project_apps(NewState, NewApps)}; -set_compile_dirs(State, Apps, {Type, Files}) when Type==spec; Type==suite -> + lists:foldl(F, {State, Apps}, Dirs); +set_compile_dirs1(State, Apps, {Type, Files}) when Type==spec; Type==suite -> %% specs or suites with dir component Dirs = find_file_dirs(Files), F = fun(Dir, {S, A}) -> maybe_inject_test_dir(S, [], A, Dir) end, - {NewState, NewApps} = lists:foldl(F, {State, Apps}, Dirs), - {ok, rebar_state:project_apps(NewState, NewApps)}. + lists:foldl(F, {State, Apps}, Dirs). find_file_dirs(Files) -> AllDirs = lists:map(fun(F) -> filename:dirname(filename:absname(F)) end, Files), @@ -507,11 +517,58 @@ copy_bare_suites(From, To) -> ok = rebar_file_utils:cp_r(SrcFiles, To), rebar_file_utils:cp_r(DataDirs, To). +maybe_copy_spec(State, [App|Apps], Spec) -> + case rebar_file_utils:path_from_ancestor(filename:dirname(Spec), rebar_app_info:dir(App)) of + {ok, []} -> + ok = rebar_file_utils:cp_r([Spec],rebar_app_info:out_dir(App)); + {ok,_} -> + ok; + {error,badparent} -> + maybe_copy_spec(State, Apps, Spec) + end; +maybe_copy_spec(State, [], Spec) -> + case rebar_file_utils:path_from_ancestor(filename:dirname(Spec), rebar_state:dir(State)) of + {ok, []} -> + ExtrasDir = filename:join([rebar_dir:base_dir(State), "extras"]), + ok = rebar_file_utils:cp_r([Spec],ExtrasDir); + _R -> + ok + end. + inject_test_dir(Opts, Dir) -> %% append specified test targets to app defined `extra_src_dirs` ExtraSrcDirs = rebar_opts:get(Opts, extra_src_dirs, []), rebar_opts:set(Opts, extra_src_dirs, ExtraSrcDirs ++ [Dir]). +get_dirs_from_specs(Specs) -> + case get_tests_from_specs(Specs) of + {ok,Tests} -> + {SpecLists,NodeRunSkipLists} = lists:unzip(Tests), + SpecList = lists:append(SpecLists), + NodeRunSkipList = lists:append(NodeRunSkipLists), + RunList = lists:append([R || {_,R,_} <- NodeRunSkipList]), + DirList = [element(1,R) || R <- RunList], + {ok,{SpecList,DirList}}; + Error -> + Error + end. + +get_tests_from_specs(Specs) -> + case erlang:function_exported(ct_testspec,get_tests,1) of + true -> + ct_testspec:get_tests(Specs); + false -> + case ct_testspec:collect_tests_from_file(Specs,true) of + Tests when is_list(Tests) -> + {ok,[{S,ct_testspec:prepare_tests(R)} || {S,R} <- Tests]}; + R when is_tuple(R), element(1,R)==testspec -> + %% R15 + {ok,[{Specs,ct_testspec:prepare_tests(R)}]}; + Error -> + Error + end + end. + translate_paths(State, Opts) -> case proplists:get_value(spec, Opts) of undefined -> -- cgit v1.1 From 998c6756b73ec2c227ed6f7f2e66eb059d1027fc Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Thu, 22 Dec 2016 20:25:52 +0100 Subject: Make sure ct_testspec is loaded ... before calling erlang:function_exported(ct_testspec,get_tests,1). --- src/rebar_prv_common_test.erl | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 4a9ba25..30596da 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -554,6 +554,7 @@ get_dirs_from_specs(Specs) -> end. get_tests_from_specs(Specs) -> + _ = ct_testspec:module_info(), % make sure ct_testspec is loaded case erlang:function_exported(ct_testspec,get_tests,1) of true -> ct_testspec:get_tests(Specs); -- cgit v1.1 From 146f2732b96b5db6476e3b86b13e7a3d7b1b2dc5 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Thu, 22 Dec 2016 22:14:07 +0100 Subject: Handle errors from ct_testspec --- src/rebar_prv_common_test.erl | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/rebar_prv_common_test.erl b/src/rebar_prv_common_test.erl index 30596da..2ac8fc7 100644 --- a/src/rebar_prv_common_test.erl +++ b/src/rebar_prv_common_test.erl @@ -100,7 +100,9 @@ format_error({badconfig, Msg}) -> io_lib:format(Msg, []); format_error({multiple_errors, Errors}) -> io_lib:format(lists:concat(["Error running tests:"] ++ - lists:map(fun(Error) -> "~n " ++ Error end, Errors)), []). + lists:map(fun(Error) -> "~n " ++ Error end, Errors)), []); +format_error({error_reading_testspec, Reason}) -> + io_lib:format("Error reading testspec: ~p", [Reason]). %% =================================================================== %% Internal functions @@ -549,8 +551,8 @@ get_dirs_from_specs(Specs) -> RunList = lists:append([R || {_,R,_} <- NodeRunSkipList]), DirList = [element(1,R) || R <- RunList], {ok,{SpecList,DirList}}; - Error -> - Error + {error,Reason} -> + {error,{?MODULE,{error_reading_testspec,Reason}}} end. get_tests_from_specs(Specs) -> -- cgit v1.1