From 610280492bca33c851eabda0a2daf5cf141ea090 Mon Sep 17 00:00:00 2001 From: Siri Hansen Date: Fri, 20 Jan 2012 17:10:31 +0100 Subject: [reltool] Add test case for reltool_server:get_mod OTP-9794 This test case revealed a bug that occurs when calling reltool_server:get_mod after reltool_server:undo_config. get_mod reads from the module table (ets) and not from the reltool_server state, while undo_config only changes the state. This bug has been corrected, so undo_config now updates both state and tables (it does the same as set_sys). --- lib/reltool/src/reltool_server.erl | 30 ++++++-- lib/reltool/src/reltool_sys_win.erl | 25 +++++-- lib/reltool/test/reltool_server_SUITE.erl | 116 ++++++++++++++++++++++-------- 3 files changed, 133 insertions(+), 38 deletions(-) diff --git a/lib/reltool/src/reltool_server.erl b/lib/reltool/src/reltool_server.erl index fdb9e08a90..7dc2d21158 100644 --- a/lib/reltool/src/reltool_server.erl +++ b/lib/reltool/src/reltool_server.erl @@ -266,12 +266,30 @@ loop(#state{common = C, sys = Sys} = S) -> reltool_utils:reply(ReplyTo, Ref, Status3), ?MODULE:loop(S6); {call, ReplyTo, Ref, undo_config} -> - S2 = S#state{sys = S#state.old_sys, - old_sys = S#state.sys, - status = S#state.old_status, - old_status = S#state.status}, - reltool_utils:reply(ReplyTo, Ref, ok), - ?MODULE:loop(S2); + OldSys = S#state.old_sys, + S2 = S#state{sys = OldSys, + old_sys = Sys}, + %%! Possibly skip old_status here, since we do all job again! + %%! If so, consider if it is correct to use Force or not - + %%! since warnings from refresh_app will not re-appear here + %%! in undo if Force==false. + Force = + (OldSys#sys.root_dir =/= Sys#sys.root_dir) orelse + (OldSys#sys.lib_dirs =/= Sys#sys.lib_dirs) orelse + (OldSys#sys.escripts =/= Sys#sys.escripts), + + {S3, Status} = refresh(S2, Force, S#state.old_status), + {S4, Status2} = analyse(S3, Status), + S5 = + case Status2 of + {ok, _Warnings} -> % BUGBUG: handle warnings + S4#state{status = Status2, old_status = S#state.status}; + {error, _} -> + %% Keep old state + S + end, + reltool_utils:reply(ReplyTo, Ref, Status2), + ?MODULE:loop(S5); {call, ReplyTo, Ref, {get_rel, RelName}} -> Sys = S#state.sys, Reply = diff --git a/lib/reltool/src/reltool_sys_win.erl b/lib/reltool/src/reltool_sys_win.erl index 8b0f64eb45..f29a95af38 100644 --- a/lib/reltool/src/reltool_sys_win.erl +++ b/lib/reltool/src/reltool_sys_win.erl @@ -1,7 +1,7 @@ %% %% %CopyrightBegin% %% -%% Copyright Ericsson AB 2009-2011. All Rights Reserved. +%% Copyright Ericsson AB 2009-2012. All Rights Reserved. %% %% The contents of this file are subject to the Erlang Public License, %% Version 1.1, (the "License"); you may not use this file except in @@ -1290,7 +1290,15 @@ reset_config(#state{status_bar = Bar} = S) -> undo_config(#state{status_bar = Bar} = S) -> wxStatusBar:setStatusText(Bar, "Processing libraries..."), - ok = reltool_server:undo_config(S#state.server_pid), + case reltool_server:undo_config(S#state.server_pid) of + {ok, []} -> + ok; + {ok, Warnings} -> + Msg = lists:flatten([[W, $\n] || W <- Warnings]), + display_message(Msg, ?wxICON_WARNING); + {error, Reason} -> + display_message(Reason, ?wxICON_ERROR) + end, refresh(S). load_config(#state{status_bar = Bar, config_file = OldFile} = S) -> @@ -1444,8 +1452,17 @@ undo_dialog(S, Warnings) -> ?wxID_OK -> true; ?wxID_CANCEL -> - reltool_server:undo_config(S#state.server_pid), - false + case reltool_server:undo_config(S#state.server_pid) of + {ok, _} -> + false; + {error, Reason} -> + Msg = "FATAL - undo failed:\n\n" ++ + Reason ++ + "\n\nTerminating...", + display_message(Msg, ?wxICON_ERROR), + io:format("~p(~p): ~s\n", [?MODULE, ?LINE, Reason]), + exit(Reason) + end end. display_message(Message, Icon) -> diff --git a/lib/reltool/test/reltool_server_SUITE.erl b/lib/reltool/test/reltool_server_SUITE.erl index 086ada41e0..79c6854465 100644 --- a/lib/reltool/test/reltool_server_SUITE.erl +++ b/lib/reltool/test/reltool_server_SUITE.erl @@ -56,7 +56,7 @@ all() -> create_script, create_target, create_embedded, create_standalone, create_old_target, otp_9135, otp_9229_exclude_app, otp_9229_exclude_mod, - get_apps, set_app_and_undo, set_apps_and_undo, + get_apps, get_mod, set_app_and_undo, set_apps_and_undo, load_config_and_undo, reset_config_and_undo, save_config, dependencies]. @@ -595,6 +595,29 @@ get_apps(_Config) -> ok. + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% +get_mod(_Config) -> + Sys = {sys,[{app,kernel,[{incl_cond,include}]}, + {app,sasl,[{incl_cond,include}]}, + {app,stdlib,[{incl_cond,include}]}, + {app,tools,[{incl_cond,derived}]}, + {app,runtime_tools,[{incl_cond,exclude}]}]}, + {ok, Pid} = ?msym({ok, _}, reltool:start_server([{config, Sys}])), + + %% Read app and get a module from the #app record + {ok,Tools} = ?msym({ok,#app{name=tools}}, reltool_server:get_app(Pid,tools)), + Cover = lists:keyfind(cover,#mod.name,Tools#app.mods), + + %% get_mod - and check that it is equal to the one in #app.mods + ?m({ok,Cover}, reltool_server:get_mod(Pid,cover)), + + ?m(ok, reltool:stop(Pid)), + + ok. + + +%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% set_app_and_undo(Config) -> Sys = {sys,[{lib_dirs,[filename:join(datadir(Config),"faulty_app_file")]}, {incl_cond, exclude}, @@ -606,25 +629,32 @@ set_app_and_undo(Config) -> {ok, Pid} = ?msym({ok, _}, reltool:start_server([{config, Sys}])), ?m({ok, Sys}, reltool:get_config(Pid)), - %% Exclude one module with set_app + %% Get app and mod {ok,Tools} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), - Mods = Tools#app.mods, - Cover = lists:keyfind(cover,#mod.name,Mods), + {ok,Cover} = ?msym({ok,#mod{name=cover, is_included=true}}, + reltool_server:get_mod(Pid,cover)), + + %% Exclude one module with set_app ExclCover = Cover#mod{incl_cond=exclude}, + Mods = Tools#app.mods, Tools1 = Tools#app{mods = lists:keyreplace(cover,#mod.name,Mods,ExclCover)}, {ok,ToolsNoCover,[]} = ?msym({ok,_,[]}, reltool_server:set_app(Pid,Tools1)), + + %% Check that the module is no longer included ?m({ok,ToolsNoCover}, reltool_server:get_app(Pid,tools)), + {ok,NoIncludeCover} = ?msym({ok,#mod{name=cover, is_included=false}}, + reltool_server:get_mod(Pid,cover)), %% Undo - ?m(ok, reltool_server:undo_config(Pid)), - ?m({ok,Tools}, reltool_server:get_app(Pid,tools)), %%! warning can come twice here... :( - ?msym({ok,["a: Cannot parse app file"++_|_]}, reltool:get_status(Pid)), + ?msym({ok,["a: Cannot parse app file"++_|_]},reltool_server:undo_config(Pid)), + ?m({ok,Tools}, reltool_server:get_app(Pid,tools)), + ?m({ok,Cover}, reltool_server:get_mod(Pid,cover)), %% Undo again, to check that it toggles - ?m(ok, reltool_server:undo_config(Pid)), + ?m({ok,[]}, reltool_server:undo_config(Pid)), ?m({ok,ToolsNoCover}, reltool_server:get_app(Pid,tools)), - ?m({ok,[]}, reltool:get_status(Pid)), + ?m({ok,NoIncludeCover}, reltool_server:get_mod(Pid,cover)), ?m(ok, reltool:stop(Pid)), ok. @@ -641,24 +671,34 @@ set_apps_and_undo(Config) -> {ok, Pid} = ?msym({ok, _}, reltool:start_server([{config, Sys}])), ?m({ok, Sys}, reltool:get_config(Pid)), - %% Exclude one application with set_apps + %% Get app and mod {ok,Tools} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), + ?m(true, Tools#app.is_pre_included), + ?m(true, Tools#app.is_included), + {ok,Cover} = ?msym({ok,#mod{name=cover, is_included=true}}, + reltool_server:get_mod(Pid,cover)), + + %% Exclude one application with set_apps ExclTools = Tools#app{incl_cond=exclude}, ?m({ok,[]}, reltool_server:set_apps(Pid,[ExclTools])), + + %% Check that the app and its modules (one of them) are no longer included {ok,NoTools} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), ?m(false, NoTools#app.is_pre_included), ?m(false, NoTools#app.is_included), + {ok,NoIncludeCover} = ?msym({ok,#mod{name=cover, is_included=false}}, + reltool_server:get_mod(Pid,cover)), %% Undo - ?m(ok, reltool_server:undo_config(Pid)), - ?m({ok,Tools}, reltool_server:get_app(Pid,tools)), %%! warning can come twice here... :( - ?msym({ok,["a: Cannot parse app file"++_|_]}, reltool:get_status(Pid)), + ?msym({ok,["a: Cannot parse app file"++_|_]},reltool_server:undo_config(Pid)), + ?m({ok,Tools}, reltool_server:get_app(Pid,tools)), + ?m({ok,Cover}, reltool_server:get_mod(Pid,cover)), %% Undo again, to check that it toggles - ?m(ok, reltool_server:undo_config(Pid)), + ?m({ok,[]}, reltool_server:undo_config(Pid)), ?m({ok,NoTools}, reltool_server:get_app(Pid,tools)), - ?m({ok,[]}, reltool:get_status(Pid)), + ?m({ok,NoIncludeCover}, reltool_server:get_mod(Pid,cover)), ?m(ok, reltool:stop(Pid)), ok. @@ -674,12 +714,16 @@ load_config_and_undo(Config) -> {ok, Pid} = ?msym({ok, _}, reltool:start_server([{config, Sys1}])), ?m({ok, Sys1}, reltool:get_config(Pid)), - %% Check that tools is included + %% Get app and mod {ok,Tools1} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), ?m(true, Tools1#app.is_pre_included), ?m(true, Tools1#app.is_included), + {ok,Cover1} = ?msym({ok,#mod{name=cover, + is_included=true, + is_pre_included=true}}, + reltool_server:get_mod(Pid,cover)), - %% Exclude one application with set_apps + %% Change tools from include to derived by loading new config Sys2 = {sys,[{lib_dirs,[filename:join(datadir(Config),"faulty_app_file")]}, {incl_cond, exclude}, {app,kernel,[{incl_cond,include}]}, @@ -695,16 +739,20 @@ load_config_and_undo(Config) -> {ok,Tools2} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), ?m(undefined, Tools2#app.is_pre_included), ?m(true, Tools2#app.is_included), + {ok,Cover2} = ?msym({ok,#mod{name=cover, + is_included=true, + is_pre_included=undefined}}, + reltool_server:get_mod(Pid,cover)), %% Undo - ?m(ok, reltool_server:undo_config(Pid)), + ?m({ok,[]}, reltool_server:undo_config(Pid)), ?m({ok,Tools1}, reltool_server:get_app(Pid,tools)), - ?m({ok,[]}, reltool:get_status(Pid)), + ?m({ok,Cover1}, reltool_server:get_mod(Pid,cover)), %% Undo again, to check that it toggles - ?m(ok, reltool_server:undo_config(Pid)), + ?msym({ok,["a: Cannot parse app file"++_|_]},reltool_server:undo_config(Pid)), ?m({ok,Tools2}, reltool_server:get_app(Pid,tools)), - ?msym({ok,["a: Cannot parse app file"++_]}, reltool:get_status(Pid)), + ?m({ok,Cover2}, reltool_server:get_mod(Pid,cover)), ?m(ok, reltool:stop(Pid)), ok. @@ -722,12 +770,16 @@ reset_config_and_undo(Config) -> {ok, Pid} = ?msym({ok, _}, reltool:start_server([{config, Sys1}])), ?m({ok, Sys1}, reltool:get_config(Pid)), - %% Check that tools is included + %% Get app and mod {ok,Tools1} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), ?m(true, Tools1#app.is_pre_included), ?m(true, Tools1#app.is_included), + {ok,Cover1} = ?msym({ok,#mod{name=cover, + is_included=true, + is_pre_included=true}}, + reltool_server:get_mod(Pid,cover)), - %% Exclude one application with set_apps + %% Exclude tools by loading new config Sys2 = {sys,[{incl_cond, exclude}, {app,kernel,[{incl_cond,include}]}, {app,sasl,[{incl_cond,include}]}, @@ -739,17 +791,27 @@ reset_config_and_undo(Config) -> {ok,Tools2} = ?msym({ok,_}, reltool_server:get_app(Pid,tools)), ?m(false, Tools2#app.is_pre_included), ?m(false, Tools2#app.is_included), + {ok,Cover2} = ?msym({ok,#mod{name=cover, + is_included=false, + is_pre_included=false}}, + reltool_server:get_mod(Pid,cover)), %% Reset %%! warning can come twice here... :( ?msym({ok,["a: Cannot parse app file"++_|_]}, reltool_server:reset_config(Pid)), ?m({ok,Tools1}, reltool_server:get_app(Pid,tools)), + ?m({ok,Cover1}, reltool_server:get_mod(Pid,cover)), - %% Undo again, to check that it toggles - ?m(ok, reltool_server:undo_config(Pid)), + %% Undo + ?m({ok,[]}, reltool_server:undo_config(Pid)), ?m({ok,Tools2}, reltool_server:get_app(Pid,tools)), - ?m({ok,[]}, reltool:get_status(Pid)), + ?m({ok,Cover2}, reltool_server:get_mod(Pid,cover)), + + %% Undo again, to check that it toggles + ?msym({ok,["a: Cannot parse app file"++_|_]},reltool_server:undo_config(Pid)), + ?m({ok,Tools1}, reltool_server:get_app(Pid,tools)), + ?m({ok,Cover1}, reltool_server:get_mod(Pid,cover)), ?m(ok, reltool:stop(Pid)), ok. @@ -851,8 +913,6 @@ save_config(Config) -> %% Test includes x and derives y and z. %% dependencies(Config) -> - PrivDir = ?config(priv_dir,Config), - %% Default: all modules included => y and z are included (derived) Sys = {sys,[{lib_dirs,[filename:join(datadir(Config),"dependencies")]}, {incl_cond, exclude}, -- cgit v1.2.3