From 6668a91b365b8390d8eb369f7d6c6294711a1fef Mon Sep 17 00:00:00 2001 From: Peter Andersson Date: Thu, 1 Sep 2011 16:32:36 +0200 Subject: Various corrections and updates to improve error handling and reporting OTP-8933 --- lib/common_test/doc/src/config_file_chapter.xml | 6 +-- lib/common_test/src/ct_config.erl | 40 ++++++++--------- lib/common_test/src/ct_config_plain.erl | 24 ++++++----- lib/common_test/src/ct_config_xml.erl | 48 ++++++++++----------- lib/common_test/src/ct_framework.erl | 19 ++++++++- lib/common_test/src/ct_logs.erl | 4 +- lib/common_test/src/ct_run.erl | 57 +++++++++++++++++++------ lib/common_test/src/ct_testspec.erl | 8 +++- lib/common_test/src/ct_util.erl | 24 ++++++++--- 9 files changed, 148 insertions(+), 82 deletions(-) diff --git a/lib/common_test/doc/src/config_file_chapter.xml b/lib/common_test/doc/src/config_file_chapter.xml index 59151a73ec..6fc6638bf7 100644 --- a/lib/common_test/doc/src/config_file_chapter.xml +++ b/lib/common_test/doc/src/config_file_chapter.xml @@ -285,7 +285,7 @@ {ok, Config} - if the configuration variables are read successfully, - {error, Error, ErrorDetails} - if the callback module fails to + {error, {Error, ErrorDetails}} - if the callback module fails to proceed with the given configuration parameters. @@ -422,14 +422,14 @@ stop()-> call(Client, Request)-> case whereis(?REGISTERED_NAME) of undefined-> - {error, not_started, Request}; + {error, {not_started, Request}}; Pid-> Pid ! {Client, Request}, receive Reply-> {ok, Reply} after 4000-> - {error, timeout, Request} + {error, {timeout, Request}} end end. diff --git a/lib/common_test/src/ct_config.erl b/lib/common_test/src/ct_config.erl index 6b75937668..fc51aea7f3 100644 --- a/lib/common_test/src/ct_config.erl +++ b/lib/common_test/src/ct_config.erl @@ -204,9 +204,9 @@ get_config_file_list(Opts) -> DefaultConfigs = process_default_configs(Opts), CfgFiles = if - DefaultConfigs == []-> + DefaultConfigs == [] -> []; - true-> + true -> [{?ct_config_txt, DefaultConfigs}] end ++ process_user_configs(Opts, []), @@ -240,12 +240,12 @@ read_config_files(Opts) -> end, ConfigFiles = case lists:keyfind(config, 1, Opts) of - {config,ConfigLists}-> + {config,ConfigLists} -> lists:foldr(fun({Callback,Files}, Acc) -> AddCallback(Callback,Files) ++ Acc end,[],ConfigLists); - false-> + false -> [] end, read_config_files_int(ConfigFiles, fun store_config/3). @@ -255,7 +255,9 @@ read_config_files_int([{Callback, File}|Files], FunToSave) -> {ok, Config} -> FunToSave(Config, Callback, File), read_config_files_int(Files, FunToSave); - {error, ErrorName, ErrorDetail}-> + {error, {ErrorName, ErrorDetail}} -> + {user_error, {ErrorName, File, ErrorDetail}}; + {error, ErrorName, ErrorDetail} -> {user_error, {ErrorName, File, ErrorDetail}} end; read_config_files_int([], _FunToSave) -> @@ -283,7 +285,7 @@ rewrite_config(Config, Callback, File) -> config=File,_='_'}), Updater = fun({Key, Value}) -> case keyfindall(Key, #ct_conf.key, OldRows) of - []-> + [] -> ets:insert(?attr_table, #ct_conf{key=Key, value=Value, @@ -453,9 +455,9 @@ update_conf(Name, NewConfig) -> reload_conf(KeyOrName) -> case lookup_handler_for_config(KeyOrName) of - []-> + [] -> undefined; - HandlerList-> + HandlerList -> HandlerList2 = lists:usort(HandlerList), read_config_files_int(HandlerList2, fun rewrite_config/3), get_config(KeyOrName) @@ -711,13 +713,13 @@ random_bytes_1(N, Acc) -> random_bytes_1(N-1, [random:uniform(255)|Acc]). check_callback_load(Callback) -> case code:is_loaded(Callback) of - {file, _Filename}-> + {file, _Filename} -> check_exports(Callback); - false-> + false -> case code:load_file(Callback) of - {module, Callback}-> + {module, Callback} -> check_exports(Callback); - {error, Error}-> + {error, Error} -> {error, Error} end end. @@ -745,14 +747,14 @@ check_config_files(Configs) -> end, Files) end; - {error, Why}-> + {error, Why} -> {error, {callback, {Callback,Why}}} end; ({Callback, []}) -> case check_callback_load(Callback) of - {ok, Callback}-> + {ok, Callback} -> Callback:check_parameter([]); - {error, Why}-> + {error, Why} -> {error, {callback, {Callback,Why}}} end end, @@ -773,15 +775,15 @@ prepare_user_configs([], Acc, _) -> prepare_config_list(Args) -> ConfigFiles = case lists:keysearch(ct_config, 1, Args) of - {value,{ct_config,Files}}-> + {value,{ct_config,Files}} -> [{?ct_config_txt,[filename:absname(F) || F <- Files]}]; - false-> + false -> [] end, UserConfigs = case lists:keysearch(userconfig, 1, Args) of - {value,{userconfig,UserConfigFiles}}-> + {value,{userconfig,UserConfigFiles}} -> prepare_user_configs(UserConfigFiles, [], new); - false-> + false -> [] end, ConfigFiles ++ UserConfigs. diff --git a/lib/common_test/src/ct_config_plain.erl b/lib/common_test/src/ct_config_plain.erl index 3fbc8af9fb..6698332379 100644 --- a/lib/common_test/src/ct_config_plain.erl +++ b/lib/common_test/src/ct_config_plain.erl @@ -29,7 +29,7 @@ read_config(ConfigFile) -> {ok,Config} -> {ok, Config}; {error,enoent} -> - {error, config_file_error, enoent}; + {error,{config_file_error,file:format_error(enoent)}}; {error,Reason} -> Key = case application:get_env(common_test, decrypt) of @@ -45,23 +45,27 @@ read_config(ConfigFile) -> end, case Key of {error,no_crypt_file} -> - {error, config_file_error, Reason}; + {error,{config_file_error, + lists:flatten( + io_lib:format("~s",[file:format_error(Reason)]))}}; {error,CryptError} -> - {error, decrypt_file_error, CryptError}; + {error,{decrypt_file_error,CryptError}}; _ when is_list(Key) -> - case ct_config:decrypt_config_file(ConfigFile, undefined, {key,Key}) of + case ct_config:decrypt_config_file(ConfigFile, + undefined, + {key,Key}) of {ok,CfgBin} -> case read_config_terms(CfgBin) of {error,ReadFail} -> - {error, config_file_error, ReadFail}; + {error,{config_file_error,ReadFail}}; Config -> - {ok, Config} + {ok,Config} end; {error,DecryptFail} -> - {error, decrypt_config_error, DecryptFail} + {error,{decrypt_config_error,DecryptFail}} end; _ -> - {error, bad_decrypt_key, Key} + {error,{bad_decrypt_key,Key}} end end. @@ -69,9 +73,9 @@ read_config(ConfigFile) -> check_parameter(File)-> case filelib:is_file(File) of true-> - {ok, {file, File}}; + {ok,{file,File}}; false-> - {error, {nofile, File}} + {error,{nofile,File}} end. read_config_terms(Bin) when is_binary(Bin) -> diff --git a/lib/common_test/src/ct_config_xml.erl b/lib/common_test/src/ct_config_xml.erl index 8a6e75e635..794174e663 100644 --- a/lib/common_test/src/ct_config_xml.erl +++ b/lib/common_test/src/ct_config_xml.erl @@ -27,30 +27,30 @@ % read config file read_config(ConfigFile) -> case catch do_read_xml_config(ConfigFile) of - {ok, Config}-> - {ok, Config}; - {error, Error, ErroneousString}-> - {error, Error, ErroneousString} + {ok,Config} -> + {ok,Config}; + Error = {error,_} -> + Error end. % check file exists -check_parameter(File)-> +check_parameter(File) -> case filelib:is_file(File) of - true-> - {ok, {file, File}}; - false-> - {error, {nofile, File}} + true -> + {ok,{file,File}}; + false -> + {error,{nofile,File}} end. % actual reading of the config -do_read_xml_config(ConfigFile)-> +do_read_xml_config(ConfigFile) -> case catch xmerl_sax_parser:file(ConfigFile, - [{event_fun, fun event/3}, - {event_state, []}]) of - {ok, EntityList, _}-> - {ok, lists:reverse(transform_entity_list(EntityList))}; - Oops-> - {error, parsing_failed, Oops} + [{event_fun,fun event/3}, + {event_state,[]}]) of + {ok,EntityList,_} -> + {ok,lists:reverse(transform_entity_list(EntityList))}; + Oops -> + {error,{parsing_failed,Oops}} end. % event callback for xmerl_sax_parser @@ -92,18 +92,18 @@ tag(_El, State) -> State. % transform of the ugly deeply nested entity list to the key-value "tree" -transform_entity_list(EntityList)-> +transform_entity_list(EntityList) -> lists:map(fun transform_entity/1, EntityList). % transform entity from {list(), list()} to {atom(), term()} transform_entity({Tag, [Value|Rest]}) when - is_tuple(Value)-> + is_tuple(Value) -> {list_to_atom(Tag), transform_entity_list(lists:reverse([Value|Rest]))}; -transform_entity({Tag, String})-> +transform_entity({Tag, String}) -> case list_to_term(String) of - {ok, Value}-> + {ok, Value} -> {list_to_atom(Tag), Value}; - Error-> + Error -> throw(Error) end. @@ -111,8 +111,8 @@ transform_entity({Tag, String})-> list_to_term(String) -> {ok, T, _} = erl_scan:string(String++"."), case catch erl_parse:parse_term(T) of - {ok, Term} -> - {ok, Term}; + {ok,Term} -> + {ok,Term}; Error -> - {error, Error, String} + {error,{Error,String}} end. diff --git a/lib/common_test/src/ct_framework.erl b/lib/common_test/src/ct_framework.erl index b730f1e46e..29caa27d94 100644 --- a/lib/common_test/src/ct_framework.erl +++ b/lib/common_test/src/ct_framework.erl @@ -245,7 +245,12 @@ add_defaults(Mod,Func,FuncInfo,DoInit) -> Error = {error,_} -> {SuiteInfo,Error}; MergedInfo -> {SuiteInfo,MergedInfo} end; - {'EXIT',Reason} -> + {'EXIT',Reason} -> + ErrStr = io_lib:format("~n*** ERROR *** " + "~w:suite/0 failed: ~p~n", + [Mod,Reason]), + io:format(ErrStr, []), + io:format(user, ErrStr, []), {suite0_failed,{exited,Reason}}; SuiteInfo when is_list(SuiteInfo) -> case lists:all(fun(E) when is_tuple(E) -> true; @@ -261,9 +266,19 @@ add_defaults(Mod,Func,FuncInfo,DoInit) -> MergedInfo -> {SuiteInfo1,MergedInfo} end; false -> + ErrStr = io_lib:format("~n*** ERROR *** " + "Invalid return value from " + "~w:suite/0: ~p~n", [Mod,SuiteInfo]), + io:format(ErrStr, []), + io:format(user, ErrStr, []), {suite0_failed,bad_return_value} end; - _ -> + SuiteInfo -> + ErrStr = io_lib:format("~n*** ERROR *** " + "Invalid return value from " + "~w:suite/0: ~p~n", [Mod,SuiteInfo]), + io:format(ErrStr, []), + io:format(user, ErrStr, []), {suite0_failed,bad_return_value} end. diff --git a/lib/common_test/src/ct_logs.erl b/lib/common_test/src/ct_logs.erl index 761b906392..f34eb83afa 100644 --- a/lib/common_test/src/ct_logs.erl +++ b/lib/common_test/src/ct_logs.erl @@ -484,8 +484,8 @@ logger_loop(State) -> [Str,Args]), %% stop the testcase, we need %% to see the fault - exit(Pid,logging_failed), - ok; + exit(Pid,{log_printout_error,Str,Args}), + []; IoStr when IoList == [] -> [IoStr]; IoStr -> diff --git a/lib/common_test/src/ct_run.erl b/lib/common_test/src/ct_run.erl index c3b0348b26..4ff062c2fa 100644 --- a/lib/common_test/src/ct_run.erl +++ b/lib/common_test/src/ct_run.erl @@ -667,6 +667,16 @@ run_test(StartOpt) when is_tuple(StartOpt) -> run_test([StartOpt]); run_test(StartOpts) when is_list(StartOpts) -> + CTPid = spawn(fun() -> run_test1(StartOpts) end), + Ref = monitor(process, CTPid), + receive + {'DOWN',Ref,process,CTPid,{user_error,Error}} -> + Error; + {'DOWN',Ref,process,CTPid,Other} -> + Other + end. + +run_test1(StartOpts) when is_list(StartOpts) -> case proplists:get_value(refresh_logs, StartOpts) of undefined -> Tracing = start_trace(StartOpts), @@ -675,7 +685,7 @@ run_test(StartOpts) when is_list(StartOpts) -> Res = case ct_repeat:loop_test(func, StartOpts) of false -> - case catch run_test1(StartOpts) of + case catch run_test2(StartOpts) of {'EXIT',Reason} -> file:set_cwd(Cwd), {error,Reason}; @@ -686,13 +696,13 @@ run_test(StartOpts) when is_list(StartOpts) -> Result end, stop_trace(Tracing), - Res; + exit(Res); RefreshDir -> refresh_logs(?abs(RefreshDir)), - ok + exit(done) end. -run_test1(StartOpts) -> +run_test2(StartOpts) -> %% label Label = get_start_opt(label, fun(Lbl) when is_list(Lbl) -> Lbl; (Lbl) when is_atom(Lbl) -> atom_to_list(Lbl) @@ -981,7 +991,7 @@ run_dir(Opts = #opts{logdir = LogDir, {Dir=[Hd|_],undefined,[]} when is_list(Dir) and is_integer(Hd) -> reformat_result(catch do_run(tests(Dir), [], Opts1, StartOpts)); - {Dir,undefined,[]} when is_atom(Dir) -> + {Dir,undefined,[]} when is_atom(Dir) and (Dir /= undefined) -> reformat_result(catch do_run(tests(atom_to_list(Dir)), [], Opts1, StartOpts)); @@ -1052,6 +1062,9 @@ run_dir(Opts = #opts{logdir = LogDir, end end; + {undefined,undefined,[]} -> + exit(no_test_specified); + {Dir,Suite,GsAndCs} -> exit({incorrect_start_options,{Dir,Suite,GsAndCs}}) end. @@ -1064,19 +1077,38 @@ run_dir(Opts = #opts{logdir = LogDir, %%% the same as those used in test specification files. %%% @equiv ct:run_testspec/1 %%%----------------------------------------------------------------- - run_testspec(TestSpec) -> + CTPid = spawn(fun() -> run_testspec1(TestSpec) end), + Ref = monitor(process, CTPid), + receive + {'DOWN',Ref,process,CTPid,{user_error,Error}} -> + Error; + {'DOWN',Ref,process,CTPid,Other} -> + Other + end. + +run_testspec1(TestSpec) -> {ok,Cwd} = file:get_cwd(), io:format("~nCommon Test starting (cwd is ~s)~n~n", [Cwd]), - case catch run_testspec1(TestSpec) of + case catch run_testspec2(TestSpec) of {'EXIT',Reason} -> file:set_cwd(Cwd), - {error,Reason}; + exit({error,Reason}); Result -> - Result + exit(Result) end. -run_testspec1(TestSpec) -> +run_testspec2(File) when is_list(File), is_integer(hd(File)) -> + case file:read_file_info(File) of + {ok,_} -> + exit("Bad argument, " + "use ct:run_test([{spec," ++ File ++ "}])"); + _ -> + exit("Bad argument, list of tuples expected, " + "use ct:run_test/1 for test specification files") + end; + +run_testspec2(TestSpec) -> case catch ct_testspec:collect_tests_from_list(TestSpec, false) of {E,CTReason} when E == error ; E == 'EXIT' -> exit(CTReason); @@ -1295,7 +1327,6 @@ do_run(Tests, Misc, LogDir, LogOpts) when is_list(Misc), do_run(Tests, Skip, Opts, Args) when is_record(Opts, opts) -> #opts{label = Label, profile = Profile, cover = Cover} = Opts, - %% label - used by ct_logs TestLabel = if Label == undefined -> undefined; @@ -2478,8 +2509,8 @@ start_trace(Args) -> false end; {_,Error} -> - io:format("Warning! Tracing not started. Reason: ~p~n~n", - [Error]), + io:format("Warning! Tracing not started. Reason: ~s~n~n", + [file:format_error(Error)]), false end; false -> diff --git a/lib/common_test/src/ct_testspec.erl b/lib/common_test/src/ct_testspec.erl index bf1dfa24ea..2cba1d8410 100644 --- a/lib/common_test/src/ct_testspec.erl +++ b/lib/common_test/src/ct_testspec.erl @@ -249,11 +249,15 @@ collect_tests_from_file1([Spec|Specs],TestSpec,Relaxed) -> SpecDir = filename:dirname(filename:absname(Spec)), case file:consult(Spec) of {ok,Terms} -> - TestSpec1 = collect_tests(Terms,TestSpec#testspec{spec_dir=SpecDir}, + TestSpec1 = collect_tests(Terms, + TestSpec#testspec{spec_dir=SpecDir}, Relaxed), collect_tests_from_file1(Specs,TestSpec1,Relaxed); {error,Reason} -> - throw({error,{Spec,Reason}}) + ReasonStr = + lists:flatten(io_lib:format("~s", + [file:format_error(Reason)])), + throw({error,{Spec,ReasonStr}}) end; collect_tests_from_file1([],TS=#testspec{config=Cfgs,event_handler=EvHs, include=Incl,tests=Tests},_) -> diff --git a/lib/common_test/src/ct_util.erl b/lib/common_test/src/ct_util.erl index ef94c25364..f393ea103d 100644 --- a/lib/common_test/src/ct_util.erl +++ b/lib/common_test/src/ct_util.erl @@ -124,13 +124,15 @@ do_start(Parent,Mode,LogDir) -> ok -> ok; E -> exit(E) end, + DoExit = fun(Reason) -> file:set_cwd(StartDir), exit(Reason) end, Opts = case read_opts() of {ok,Opts1} -> Opts1; Error -> Parent ! {self(),Error}, - exit(Error) + DoExit(Error) end, + %% start an event manager (if not already started by master) case ct_event:start_link() of {error,{already_started,_}} -> @@ -143,16 +145,23 @@ do_start(Parent,Mode,LogDir) -> ct_event:add_handler([{vts,VtsPid}]) end end, + %% start ct_config server - ct_config:start(Mode), + try ct_config:start(Mode) of + _ -> ok + catch + _Class:CfgError -> + DoExit(CfgError) + end, + %% add user event handlers case lists:keysearch(event_handler,1,Opts) of {value,{_,Handlers}} -> Add = fun({H,Args}) -> case catch gen_event:add_handler(?CT_EVMGR_REF,H,Args) of ok -> ok; - {'EXIT',Why} -> exit(Why); - Other -> exit({event_handler,Other}) + {'EXIT',Why} -> DoExit(Why); + Other -> DoExit({event_handler,Other}) end end, case catch lists:foreach(Add,Handlers) of @@ -171,10 +180,11 @@ do_start(Parent,Mode,LogDir) -> data={StartTime, lists:flatten(TestLogDir)}}), %% Initialize ct_hooks - case catch ct_hooks:init(Opts) of + try ct_hooks:init(Opts) of ok -> - Parent ! {self(),started}; - {_,CTHReason} -> + Parent ! {self(),started} + catch + _:CTHReason -> ct_logs:tc_print('Suite Callback',CTHReason,[]), self() ! {{stop,{self(),{user_error,CTHReason}}}, {Parent,make_ref()}} -- cgit v1.2.3