Merge pull request #60 from rebar/test-packages

Test packages

7 files changed, 297 insertions, 61 deletions

diff --git a/src/rebar_digraph.erl b/src/rebar_digraph.erl
index dbcf649..55d7272 100644
--- a/src/rebar_digraph.erl
+++ b/src/rebar_digraph.erl
@@ -69,27 +69,31 @@ restore_graph({Vs, Es}) ->
 %% The first dep while traversing the graph is chosen and any conflicting
 %% dep encountered later on is ignored.
 cull_deps(Graph, Vertices) ->
-    cull_deps(Graph, Vertices, lists:foldl(fun({Key, _}=N, Solution) ->
-                                               dict:store(Key, N, Solution)
-                                       end, dict:new(), Vertices)).
+    cull_deps(Graph,
+              Vertices,
+              lists:foldl(fun({Key, _}=N, Solution) -> dict:store(Key, N, Solution) end,
+                          dict:new(), Vertices),
+              []).
 
-cull_deps(_Graph, [], Solution) ->
+cull_deps(_Graph, [], Solution, Discarded) ->
     {_, Vertices} = lists:unzip(dict:to_list(Solution)),
-    {ok, Vertices};
-cull_deps(Graph, Vertices, Solution) ->
-    {NV, NS} =
-        lists:foldl(fun(V, {NewVertices, SolutionAcc}) ->
+    {ok, Vertices, Discarded};
+cull_deps(Graph, Vertices, Solution, Discarded) ->
+    {NV, NS, DS} =
+        lists:foldl(fun(V, {NewVertices, SolutionAcc, DiscardedAcc}) ->
                         OutNeighbors = digraph:out_neighbours(Graph, V),
-                        lists:foldl(fun({Key, _}=N, {NewVertices1, SolutionAcc1}) ->
-                                            case dict:is_key(Key, SolutionAcc1) of
-                                                true ->
-                                                    {NewVertices1, SolutionAcc1};
-                                                false ->
-                                                    {[N | NewVertices1], dict:store(Key, N, SolutionAcc1)}
+                        lists:foldl(fun({Key, _}=N, {NewVertices1, SolutionAcc1, DiscardedAcc1}) ->
+                                            case dict:find(Key, SolutionAcc1) of
+                                                {ok, N} -> % already seen
+                                                    {NewVertices1, SolutionAcc1, DiscardedAcc1};
+                                                {ok, _} -> % conflict resolution!
+                                                    {NewVertices1, SolutionAcc1, [N|DiscardedAcc1]};
+                                                error ->
+                                                    {[N | NewVertices1], dict:store(Key, N, SolutionAcc1), DiscardedAcc1}
                                             end
-                                    end, {NewVertices, SolutionAcc}, OutNeighbors)
-                    end, {[], Solution}, Vertices),
-    cull_deps(Graph, NV, NS).
+                                    end, {NewVertices, SolutionAcc, DiscardedAcc}, OutNeighbors)
+                    end, {[], Solution, Discarded}, lists:sort(Vertices)),
+    cull_deps(Graph, NV, NS, DS).
 
 subgraph(Graph, Vertices) ->
     digraph_utils:subgraph(Graph, Vertices).
diff --git a/src/rebar_packages.erl b/src/rebar_packages.erl
index 2164b79..30d11c9 100644
--- a/src/rebar_packages.erl
+++ b/src/rebar_packages.erl
@@ -22,7 +22,7 @@ get_packages(State) ->
                 {Dict, rebar_digraph:restore_graph(Graph)}
             catch
                 _:_ ->
-                    ?ERROR("Bad packages index, try to fix with `rebar update`", []),
+                    ?ERROR("Bad packages index, try to fix with `rebar3 update`", []),
                     {dict:new(), digraph:new()}
             end;
         false ->
diff --git a/src/rebar_prv_install_deps.erl b/src/rebar_prv_install_deps.erl
index 5878e8c..3728b70 100644
--- a/src/rebar_prv_install_deps.erl
+++ b/src/rebar_prv_install_deps.erl
@@ -81,15 +81,20 @@ do(State) ->
                                 end,
 
         Source = ProjectApps ++ SrcApps,
-        case rebar_digraph:compile_order(Source) of
-            {ok, Sort} ->
-                {ok, rebar_state:deps_to_build(State1,
-                                              lists:dropwhile(fun rebar_app_info:valid/1
-                                                             , Sort -- ProjectApps))};
-            {error, {cycles, Cycles}} ->
+        case find_cycles(Source ++ rebar_state:all_deps(State1)) of
+            {cycles, Cycles} ->
                 ?PRV_ERROR({cycles, Cycles});
             {error, Error} ->
-                {error, Error}
+                {error, Error};
+            no_cycle ->
+                case rebar_digraph:compile_order(Source) of
+                    {ok, Sort} ->
+                        {ok, rebar_state:deps_to_build(State1,
+                                                    lists:dropwhile(fun rebar_app_info:valid/1,
+                                                                    Sort -- ProjectApps))};
+                    {error, Error} ->
+                        {error, Error}
+                end
         end
     catch
         %% maybe_fetch will maybe_throw an exception to break out of some loops
@@ -97,6 +102,13 @@ do(State) ->
             {error, Reason}
     end.
 
+find_cycles(Apps) ->
+    case rebar_digraph:compile_order(Apps) of
+        {error, {cycles, Cycles}} -> {cycles, Cycles};
+        {error, Error} -> {error, Error};
+        {ok, _} -> no_cycle
+    end.
+
 -spec format_error(any()) -> iolist().
 format_error({cycles, Cycles}) ->
     Prints = [["applications: ",
@@ -119,11 +131,9 @@ handle_deps(State, [], _) ->
 handle_deps(State, Deps, Update) ->
     %% Read in package index and dep graph
     {Packages, Graph} = rebar_packages:get_packages(State),
-
     %% Split source deps from pkg deps, needed to keep backwards compatibility
     DepsDir = rebar_dir:deps_dir(State),
     {SrcDeps, PkgDeps} = parse_deps(State, DepsDir, Deps),
-
     %% Fetch transitive src deps
     {State1, SrcApps, PkgDeps1, Seen} =
         update_src_deps(0, SrcDeps, PkgDeps, [], State, Update, sets:new()),
@@ -134,15 +144,16 @@ handle_deps(State, Deps, Update) ->
                            PkgDeps2 ->
                                %% Find pkg deps needed
                                S = case rebar_digraph:cull_deps(Graph, PkgDeps2) of
-                                       {ok, []} ->
+                                       {ok, [], _} ->
                                            throw({rebar_digraph, no_solution});
-                                       {ok, Solution} ->
+                                       {ok, Solution, []} ->
+                                           Solution;
+                                       {ok, Solution, Discarded} ->
+                                           [warn_skip_pkg(Pkg) || Pkg <- Discarded],
                                            Solution
                                    end,
-
                                update_pkg_deps(S, Packages, Update, Seen, State1)
                        end,
-
     AllDeps = lists:ukeymerge(2
                              ,lists:ukeysort(2, SrcApps)
                              ,lists:ukeysort(2, Solved)),
@@ -193,7 +204,8 @@ package_to_app(DepsDir, Packages, {Name, Vsn}) ->
         error ->
             {error, missing_package};
         {ok, P} ->
-            PkgDeps = proplists:get_value(<<"deps">>, P, []),
+            PkgDeps = [{atom_to_binary(PkgName,utf8), list_to_binary(PkgVsn)}
+                       || {PkgName,PkgVsn} <- proplists:get_value(<<"deps">>, P, [])],
             Link = proplists:get_value(<<"link">>, P, ""),
             {ok, AppInfo} = rebar_app_info:new(Name, Vsn),
             AppInfo1 = rebar_app_info:deps(AppInfo, PkgDeps),
@@ -233,7 +245,9 @@ update_src_deps(Level, SrcDeps, PkgDeps, SrcApps, State, Update, Seen) ->
                                          end,
                                      {SrcDepsAcc1, PkgDepsAcc1, SrcAppsAcc1, StateAcc2, SeenAcc1}
                              end
-                     end, {[], PkgDeps, SrcApps, State, Seen}, SrcDeps) of
+                     end,
+                     {[], PkgDeps, SrcApps, State, Seen},
+                     lists:sort(SrcDeps)) of
         {[], NewPkgDeps, NewSrcApps, State1, Seen1} ->
             {State1, NewSrcApps, NewPkgDeps, Seen1};
         {NewSrcDeps, NewPkgDeps, NewSrcApps, State1, Seen1} ->
@@ -427,3 +441,8 @@ warn_skip_deps(AppInfo) ->
           "has already been fetched~n",
           [rebar_app_info:name(AppInfo),
            rebar_app_info:source(AppInfo)]).
+
+warn_skip_pkg({Name, Source}) ->
+    ?WARN("Skipping ~s (version ~s from package index) as an app of the same "
+          "name has already been fetched~n",
+          [Name, Source]).
diff --git a/test/mock_git_resource.erl b/test/mock_git_resource.erl
index d867a28..00f0a03 100644
--- a/test/mock_git_resource.erl
+++ b/test/mock_git_resource.erl
@@ -105,7 +105,7 @@ mock_download(Opts) ->
             filelib:ensure_dir(Dir),
             {git, Url, {_, Vsn}} = normalize_git(Git, Overrides, Default),
             App = app(Url),
-            AppDeps = proplists:get_value(App, Deps, []),
+            AppDeps = proplists:get_value({App,Vsn}, Deps, []),
             rebar_test_utils:create_app(
                 Dir, App, Vsn,
                 [element(1,D) || D  <- AppDeps]
diff --git a/test/mock_pkg_resource.erl b/test/mock_pkg_resource.erl
new file mode 100644
index 0000000..502e184
--- /dev/null
+++ b/test/mock_pkg_resource.erl
@@ -0,0 +1,143 @@
+%%% Mock a package resource and create an app magically for each URL submitted.
+-module(mock_pkg_resource).
+-export([mock/0, mock/1, unmock/0]).
+-define(MOD, rebar_pkg_resource).
+
+%%%%%%%%%%%%%%%%%
+%%% Interface %%%
+%%%%%%%%%%%%%%%%%
+
+%% @doc same as `mock([])'.
+mock() -> mock([]).
+
+%% @doc Mocks a fake version of the git resource fetcher that creates
+%% empty applications magically, rather than trying to download them.
+%% Specific config options are explained in each of the private functions.
+-spec mock(Opts) -> ok when
+    Opts :: [Option],
+    Option :: {update, [App]}
+            | {default_vsn, Vsn}
+            | {override_vsn, [{App, Vsn}]}
+            | {not_in_index, [{App, Vsn}]}
+            | {pkgdeps, [{{App,Vsn}, [Dep]}]},
+    App :: string(),
+    Dep :: {App, string(), {pkg, App, Vsn, Url::string()}},
+    Vsn :: string().
+mock(Opts) ->
+    meck:new(?MOD, [no_link]),
+    mock_lock(Opts),
+    mock_update(Opts),
+    mock_vsn(Opts),
+    mock_download(Opts),
+    mock_pkg_index(Opts),
+    ok.
+
+unmock() ->
+    meck:unload(?MOD),
+    meck:unload(rebar_packages).
+
+%%%%%%%%%%%%%%%
+%%% Private %%%
+%%%%%%%%%%%%%%%
+
+%% @doc creates values for a lock file.
+mock_lock(_) ->
+    meck:expect(?MOD, lock, fun(_AppDir, Source) -> Source end).
+
+%% @doc The config passed to the `mock/2' function can specify which apps
+%% should be updated on a per-name basis: `{update, ["App1", "App3"]}'.
+mock_update(Opts) ->
+    ToUpdate = proplists:get_value(update, Opts, []),
+    meck:expect(
+        ?MOD, needs_update,
+        fun(_Dir, {pkg, App, _Vsn, _Url}) ->
+            lists:member(App, ToUpdate)
+        end).
+
+%% @doc Replicated an unsupported call.
+mock_vsn(_Opts) ->
+    meck:expect(
+        ?MOD, make_vsn,
+        fun(_Dir) ->
+            {error, "Replacing version of type pkg not supported."}
+        end).
+
+%% @doc For each app to download, create a dummy app on disk instead.
+%% The configuration for this one (passed in from `mock/1') includes:
+%%
+%% - Specify a version with `{pkg, _, Vsn, _}'
+%% - Dependencies for each application must be passed of the form:
+%%   `{pkgdeps, [{"app1", [{app2, ".*", {pkg, ...}}]}]}' -- basically
+%%   the `pkgdeps' option takes a key/value list of terms to output directly
+%%   into a `rebar.config' file to describe dependencies.
+mock_download(Opts) ->
+    Deps = proplists:get_value(pkgdeps, Opts, []),
+    meck:expect(
+        ?MOD, download,
+        fun (Dir, {pkg, AppBin, Vsn, _Url}) ->
+            App = binary_to_list(AppBin),
+            filelib:ensure_dir(Dir),
+            AppDeps = proplists:get_value({App,Vsn}, Deps, []),
+            {ok, AppInfo} = rebar_test_utils:create_app(
+                Dir, App, Vsn,
+                [element(1,D) || D  <- AppDeps]
+            ),
+            rebar_test_utils:create_config(Dir, [{deps, AppDeps}]),
+            Tarball = filename:join([Dir, "package.tar.gz"]),
+            Files = all_files(rebar_app_info:dir(AppInfo)),
+            ok = erl_tar:create(Tarball,
+                                archive_names(Dir, App, Vsn, Files),
+                                [compressed]),
+            [file:delete(F) || F <- Files],
+            {tarball, Tarball}
+        end).
+
+%% @doc On top of the pkg resource mocking, we need to mock the package
+%% index.
+%%
+%% A special option, `{not_in_index, [App]}' lets the index leave out
+%% specific applications otherwise listed.
+mock_pkg_index(Opts) ->
+    Deps = proplists:get_value(pkgdeps, Opts, []),
+    Skip = proplists:get_value(not_in_index, Opts, []),
+    %% Dict: {App, Vsn}: [{<<"link">>, <<>>}, {<<"deps">>, []}]
+    %% Digraph: all apps and deps in the index
+    Dict = find_parts(Deps, Skip),
+    GraphParts = to_graph_parts(Dict),
+    Digraph = rebar_digraph:restore_graph(GraphParts),
+    meck:new(rebar_packages, [passthrough, no_link]),
+    meck:expect(rebar_packages, get_packages,
+                fun(_State) -> {Dict, Digraph} end).
+
+
+%%%%%%%%%%%%%%%
+%%% Helpers %%%
+%%%%%%%%%%%%%%%
+all_files(Dir) ->
+    filelib:wildcard(filename:join([Dir, "**"])).
+
+archive_names(Dir, App, Vsn, Files) ->
+    ArchName = App++"-"++binary_to_list(Vsn),
+    [{ArchName ++ (F -- Dir), F} || F <- Files].
+
+find_parts(Apps, Skip) -> find_parts(Apps, Skip, dict:new()).
+
+find_parts([], _, Acc) -> Acc;
+find_parts([{AppName, Deps}|Rest], Skip, Acc) ->
+    case lists:member(AppName, Skip) orelse dict:is_key(AppName,Acc) of
+        true -> find_parts(Rest, Skip, Acc);
+        false ->
+            AccNew = dict:store(AppName,
+                                [{<<"deps">>,Deps}, {<<"link">>,<<"undef">>}],
+                                Acc),
+            find_parts(Rest, Skip, AccNew)
+    end.
+
+to_graph_parts(Dict) ->
+    LastUpdated = now(),
+    dict:fold(fun(K,V,{Ks,Vs}) ->
+            {_,Deps} = lists:keyfind(<<"deps">>, 1, V),
+            {[{K,LastUpdated}|Ks],
+             [{K,{list_to_binary(atom_to_list(DK)), list_to_binary(DV)}}
+             || {DK,DV} <- Deps]  ++ Vs}
+        end, {[],[]}, Dict).
diff --git a/test/rebar_deps_SUITE.erl b/test/rebar_deps_SUITE.erl
index ccc5fcf..c6f24c0 100644
--- a/test/rebar_deps_SUITE.erl
+++ b/test/rebar_deps_SUITE.erl
@@ -4,8 +4,14 @@
 -include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
 
-all() -> [flat, pick_highest_left, pick_highest_right, pick_earliest,
-          circular1, circular2, circular_skip].
+all() -> [{group, git}, {group, pkg}].
+
+groups() ->
+    [{all, [], [flat, pick_highest_left, pick_highest_right,
+                pick_smallest1, pick_smallest2,
+                circular1, circular2, circular_skip]},
+     {git, [], [{group, all}]},
+     {pkg, [], [{group, all}]}].
 
 init_per_suite(Config) ->
     application:start(meck),
@@ -14,21 +20,32 @@ init_per_suite(Config) ->
 end_per_suite(_Config) ->
     application:stop(meck).
 
+init_per_group(git, Config) ->
+    [{deps_type, git} | Config];
+init_per_group(pkg, Config) ->
+    [{deps_type, pkg} | Config];
+init_per_group(_, Config) ->
+    Config.
+
+end_per_group(_, Config) ->
+    Config.
+
 init_per_testcase(Case, Config) ->
     {Deps, Warnings, Expect} = deps(Case),
     Expected = case Expect of
         {ok, List} -> {ok, format_expected_deps(List)};
         {error, Reason} -> {error, Reason}
     end,
+    DepsType = ?config(deps_type, Config),
     mock_warnings(),
     [{expect, Expected},
-     {warnings, Warnings} | setup_project(Case, Config, expand_deps(Deps))].
+     {warnings, Warnings}
+    | setup_project(Case, Config, expand_deps(DepsType, Deps))].
 
 end_per_testcase(_, Config) ->
     meck:unload(),
     Config.
 
-
 format_expected_deps(Deps) ->
     [case Dep of
         {N,V} -> {dep, N, V};
@@ -64,10 +81,17 @@ deps(pick_highest_right) ->
       {"C", [{"B", "2", []}]}],
      [{"B","2"}],
      {ok, [{"B","1"}, "C"]}};
-deps(pick_earliest) ->
+deps(pick_smallest1) ->
     {[{"B", [{"D", "1", []}]},
       {"C", [{"D", "2", []}]}],
      [{"D","2"}],
+     %% we pick D1 because B < C
+     {ok, ["B","C",{"D","1"}]}};
+deps(pick_smallest2) ->
+    {[{"C", [{"D", "2", []}]},
+      {"B", [{"D", "1", []}]}],
+     [{"D","2"}],
+     %% we pick D1 because B < C
      {ok, ["B","C",{"D","1"}]}};
 deps(circular1) ->
     {[{"B", [{"A", []}]}, % A is the top-level app
@@ -80,39 +104,78 @@ deps(circular2) ->
      [],
      {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",[]}]}],
+    %% Never spot the circular dep due to being to low in the deps tree
+    %% in source deps
+    {[{"B", [{"C", "2", [{"B", []}]}]},
+      {"C", "1", [{"D",[]}]}],
      [{"C","2"}],
      {ok, ["B", {"C","1"}, "D"]}}.
 
-expand_deps([]) -> [];
-expand_deps([{Name, Deps} | Rest]) ->
+expand_deps(_, []) -> [];
+expand_deps(git, [{Name, Deps} | Rest]) ->
     Dep = {Name, ".*", {git, "https://example.org/user/"++Name++".git", "master"}},
-    [{Dep, expand_deps(Deps)} | expand_deps(Rest)];
-expand_deps([{Name, Vsn, Deps} | Rest]) ->
+    [{Dep, expand_deps(git, Deps)} | expand_deps(git, Rest)];
+expand_deps(git, [{Name, Vsn, Deps} | Rest]) ->
     Dep = {Name, Vsn, {git, "https://example.org/user/"++Name++".git", {tag, Vsn}}},
-    [{Dep, expand_deps(Deps)} | expand_deps(Rest)].
+    [{Dep, expand_deps(git, Deps)} | expand_deps(git, Rest)];
+expand_deps(pkg, [{Name, Deps} | Rest]) ->
+    Dep = {pkg, Name, "0.0.0", "https://example.org/user/"++Name++".tar.gz"},
+    [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)];
+expand_deps(pkg, [{Name, Vsn, Deps} | Rest]) ->
+    Dep = {pkg, Name, Vsn, "https://example.org/user/"++Name++".tar.gz"},
+    [{Dep, expand_deps(pkg, Deps)} | expand_deps(pkg, Rest)].
 
 setup_project(Case, Config0, Deps) ->
-    Config = rebar_test_utils:init_rebar_state(Config0, atom_to_list(Case)),
+    DepsType = ?config(deps_type, Config0),
+    Config = rebar_test_utils:init_rebar_state(
+            Config0,
+            atom_to_list(Case)++"_"++atom_to_list(DepsType)++"_"
+    ),
     AppDir = ?config(apps, Config),
     rebar_test_utils:create_app(AppDir, "A", "0.0.0", [kernel, stdlib]),
     TopDeps = top_level_deps(Deps),
     RebarConf = rebar_test_utils:create_config(AppDir, [{deps, TopDeps}]),
-    mock_git_resource:mock([{deps, flat_deps(Deps)}]),
+    case DepsType of
+        git ->
+            mock_git_resource:mock([{deps, flat_deps(Deps)}]);
+        pkg ->
+            mock_pkg_resource:mock([{pkgdeps, flat_pkgdeps(Deps)}])
+    end,
     [{rebarconfig, RebarConf} | Config].
 
 
 flat_deps([]) -> [];
-flat_deps([{{Name,_Vsn,_Ref}, Deps} | Rest]) ->
-    [{Name, top_level_deps(Deps)}]
+flat_deps([{{Name,_Vsn,Ref}, Deps} | Rest]) ->
+    [{{Name,vsn_from_ref(Ref)}, top_level_deps(Deps)}]
     ++
     flat_deps(Deps)
     ++
     flat_deps(Rest).
 
-top_level_deps(Deps) -> [{list_to_atom(Name),Vsn,Ref} || {{Name,Vsn,Ref},_} <- Deps].
+vsn_from_ref({git, _, {_, Vsn}}) -> Vsn;
+vsn_from_ref({git, _, Vsn}) -> Vsn.
+
+flat_pkgdeps([]) -> [];
+flat_pkgdeps([{{pkg, Name, Vsn, _Url}, Deps} | Rest]) ->
+    [{{iolist_to_binary(Name),iolist_to_binary(Vsn)}, top_level_deps(Deps)}]
+    ++
+    flat_pkgdeps(Deps)
+    ++
+    flat_pkgdeps(Rest).
+
+top_level_deps([]) -> [];
+top_level_deps([{{Name, Vsn, Ref}, _} | Deps]) ->
+    [{list_to_atom(Name), Vsn, Ref} | top_level_deps(Deps)];
+top_level_deps([{{pkg, Name, Vsn, _URL}, _} | Deps]) ->
+    [{list_to_atom(Name), Vsn} | top_level_deps(Deps)].
+
+app_vsn([]) -> [];
+app_vsn([{Source, Deps} | Rest]) ->
+    {Name, Vsn} = case Source of
+        {N,V,_Ref} -> {N,V};
+        {pkg, N, V, _} -> {N,V}
+    end,
+    [{Name, Vsn}] ++ app_vsn(Deps) ++ app_vsn(Rest).
 
 mock_warnings() ->
     %% just let it do its thing, we check warnings through
@@ -123,7 +186,8 @@ mock_warnings() ->
 flat(Config) -> run(Config).
 pick_highest_left(Config) -> run(Config).
 pick_highest_right(Config) -> run(Config).
-pick_earliest(Config) -> run(Config).
+pick_smallest1(Config) -> run(Config).
+pick_smallest2(Config) -> run(Config).
 circular1(Config) -> run(Config).
 circular2(Config) -> run(Config).
 circular_skip(Config) -> run(Config).
@@ -133,21 +197,26 @@ run(Config) ->
     rebar_test_utils:run_and_check(
         Config, RebarConfig, "install_deps", ?config(expect, Config)
     ),
-    check_warnings(warning_calls(), ?config(warnings, Config)).
+    check_warnings(warning_calls(), ?config(warnings, Config), ?config(deps_type, Config)).
 
 warning_calls() ->
     History = meck:history(rebar_log),
     [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History].
 
-check_warnings(_, []) ->
+check_warnings(_, [], _) ->
     ok;
-check_warnings(Warns, [{Name, Vsn} | Rest]) ->
+check_warnings(Warns, [{Name, Vsn} | Rest], Type) ->
     ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]),
-    ?assert(in_warnings(Warns, Name, Vsn)),
-    check_warnings(Warns, Rest).
+    ?assert(in_warnings(Type, Warns, Name, Vsn)),
+    check_warnings(Warns, Rest, Type).
 
-in_warnings(Warns, NameRaw, VsnRaw) ->
+in_warnings(git, Warns, NameRaw, VsnRaw) ->
     Name = iolist_to_binary(NameRaw),
     1 =< length([1 || {_, [AppName, {git, _, {_, Vsn}}]} <- Warns,
-                      AppName =:= Name, Vsn =:= VsnRaw]).
+                      AppName =:= Name, Vsn =:= VsnRaw]);
+in_warnings(pkg, Warns, NameRaw, VsnRaw) ->
+    Name = iolist_to_binary(NameRaw),
+    Vsn = iolist_to_binary(VsnRaw),
+    1 =< length([1 || {_, [AppName, AppVsn]} <- Warns,
+                      AppName =:= Name, AppVsn =:= Vsn]).
 
diff --git a/test/rebar_test_utils.erl b/test/rebar_test_utils.erl
index ed494a5..eb11280 100644
--- a/test/rebar_test_utils.erl
+++ b/test/rebar_test_utils.erl
@@ -113,7 +113,8 @@ check_results(AppDir, Expected) ->
                     false ->
                         error({app_not_found, Name});
                     {Name, App} ->
-                        ?assertEqual(Vsn, rebar_app_info:original_vsn(App))
+                        ?assertEqual(iolist_to_binary(Vsn),
+                                     iolist_to_binary(rebar_app_info:original_vsn(App)))
                 end
         end, Expected).