From 4116d41dca67b26bef26c925722ab0cdaa850fa8 Mon Sep 17 00:00:00 2001 From: Fred Hebert Date: Sun, 7 Dec 2014 21:12:07 +0000 Subject: Print warning when deps are being skipped. The case in mind here is due to conflicts, and tests have been added for this. --- src/rebar_prv_install_deps.erl | 7 ++++ test/rebar_deps_SUITE.erl | 77 +++++++++++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl index 00ef9c5..4f8ea98 100644 --- a/src/rebar_prv_install_deps.erl +++ b/src/rebar_prv_install_deps.erl @@ -206,6 +206,7 @@ update_src_deps(Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen) -> %% If not seen, add to list of locks to write out case sets:is_element(rebar_app_info:name(AppInfo), SeenAcc) of true -> + warn_skip_deps(AppInfo), {SrcDepsAcc, PkgDepsAcc, SrcAppsAcc, StateAcc, SeenAcc}; false -> {SeenAcc1, StateAcc1} = maybe_lock(AppInfo, SeenAcc, StateAcc), @@ -419,3 +420,9 @@ info(Description) -> {app_name, ".*", {fossil, "https://www.example.org/url", "Vsn"}}, {app_name, ".*", {p4, "//depot/subdir/app_dir"}}]} ]). + +warn_skip_deps(AppInfo) -> + ?WARN("Skipping ~s (from ~p) as an app of the same name " + "has already been fetched~n", + [rebar_app_info:name(AppInfo), + rebar_app_info:source(AppInfo)]). diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl index 656c3b5..ccc5fcf 100644 --- a/test/rebar_deps_SUITE.erl +++ b/test/rebar_deps_SUITE.erl @@ -5,7 +5,7 @@ -include_lib("eunit/include/eunit.hrl"). all() -> [flat, pick_highest_left, pick_highest_right, pick_earliest, - circular1, circular2]. + circular1, circular2, circular_skip]. init_per_suite(Config) -> application:start(meck), @@ -15,12 +15,19 @@ end_per_suite(_Config) -> application:stop(meck). init_per_testcase(Case, Config) -> - {Deps, Expect} = deps(Case), + {Deps, Warnings, Expect} = deps(Case), Expected = case Expect of {ok, List} -> {ok, format_expected_deps(List)}; {error, Reason} -> {error, Reason} end, - [{expect, Expected} | setup_project(Case, Config, expand_deps(Deps))]. + mock_warnings(), + [{expect, Expected}, + {warnings, Warnings} | setup_project(Case, Config, expand_deps(Deps))]. + +end_per_testcase(_, Config) -> + meck:unload(), + Config. + format_expected_deps(Deps) -> [case Dep of @@ -28,35 +35,56 @@ format_expected_deps(Deps) -> N -> {dep, N} end || Dep <- Deps]. +%% format: +%% {Spec, +%% [Warning], +%% {ok, Result} | {error, Reason}} +%% +%% Spec is a list of levelled dependencies of two possible forms: +%% - {"Name", Spec} +%% - {"Name", "Vsn", Spec} +%% +%% Warnings are going to match on mocked ?WARN(...) +%% calls to be evaluated. An empty list means we do not care about +%% warnings, not that no warnings will be printed. This means +%% the list of warning isn't interpreted to be exhaustive, and more +%% warnings may be generated than are listed. deps(flat) -> {[{"B", []}, {"C", []}], + [], {ok, ["B", "C"]}}; deps(pick_highest_left) -> {[{"B", [{"C", "2", []}]}, {"C", "1", []}], - {ok, ["B", {"C", "1"}]}}; % Warn C2 + [{"C","2"}], + {ok, ["B", {"C", "1"}]}}; deps(pick_highest_right) -> {[{"B", "1", []}, {"C", [{"B", "2", []}]}], - {ok, [{"B","1"}, "C"]}}; % Warn B2 + [{"B","2"}], + {ok, [{"B","1"}, "C"]}}; deps(pick_earliest) -> {[{"B", [{"D", "1", []}]}, {"C", [{"D", "2", []}]}], - {ok, ["B","C",{"D","1"}]}}; % Warn D2 + [{"D","2"}], + {ok, ["B","C",{"D","1"}]}}; deps(circular1) -> {[{"B", [{"A", []}]}, % A is the top-level app {"C", []}], + [], {error, {rebar_prv_install_deps, {cycles, [[<<"A">>,<<"B">>]]}}}}; deps(circular2) -> {[{"B", [{"C", [{"B", []}]}]}, {"C", []}], - {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}}. - -end_per_testcase(_, Config) -> - mock_git_resource:unmock(), - meck:unload(), - Config. + [], + {error, {rebar_prv_install_deps, {cycles, [[<<"B">>,<<"C">>]]}}}}; +deps(circular_skip) -> + %% Never spot the circular dep due to being to low in the deps tree. + {[{"B", [{"C", "2", [{"D", []}]}]}, + {"C", "1", [{"B",[]}]}], + [{"C","2"}], + {ok, ["B", {"C","1"}, "D"]}}. expand_deps([]) -> []; expand_deps([{Name, Deps} | Rest]) -> @@ -86,6 +114,11 @@ flat_deps([{{Name,_Vsn,_Ref}, Deps} | Rest]) -> top_level_deps(Deps) -> [{list_to_atom(Name),Vsn,Ref} || {{Name,Vsn,Ref},_} <- Deps]. +mock_warnings() -> + %% just let it do its thing, we check warnings through + %% the call log. + meck:new(rebar_log, [no_link, passthrough]). + %%% TESTS %%% flat(Config) -> run(Config). pick_highest_left(Config) -> run(Config). @@ -93,10 +126,28 @@ pick_highest_right(Config) -> run(Config). pick_earliest(Config) -> run(Config). circular1(Config) -> run(Config). circular2(Config) -> run(Config). +circular_skip(Config) -> run(Config). run(Config) -> {ok, RebarConfig} = file:consult(?config(rebarconfig, Config)), rebar_test_utils:run_and_check( Config, RebarConfig, "install_deps", ?config(expect, Config) - ). + ), + check_warnings(warning_calls(), ?config(warnings, Config)). + +warning_calls() -> + History = meck:history(rebar_log), + [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History]. + +check_warnings(_, []) -> + ok; +check_warnings(Warns, [{Name, Vsn} | Rest]) -> + ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]), + ?assert(in_warnings(Warns, Name, Vsn)), + check_warnings(Warns, Rest). + +in_warnings(Warns, NameRaw, VsnRaw) -> + Name = iolist_to_binary(NameRaw), + 1 =< length([1 || {_, [AppName, {git, _, {_, Vsn}}]} <- Warns, + AppName =:= Name, Vsn =:= VsnRaw]). -- cgit v1.1