Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

format hex packages in a nice/human way #2032

Merged
merged 2 commits into from
May 6, 2019
Merged

format hex packages in a nice/human way #2032

merged 2 commits into from
May 6, 2019

Conversation

fenollp
Copy link
Contributor

@fenollp fenollp commented Mar 17, 2019

Changes

===> Verifying dependencies...
===> Upgrading lager ({pkg,<<"lager">>,<<"3.6.8">>})
===> Downloaded package, caching at /home/pete/.cache/rebar3/hex/hexpm/packages/lager-3.6.8.tar
===> No upgrade needed for amqp_client
===> No upgrade needed for jsx
===> Upgrading lager ({pkg,<<"lager">>,<<"3.6.9">>})
===> Version cached at /home/pete/.cache/rebar3/hex/hexpm/packages/lager-3.6.9.tar is up to date, reusing it

into

===> Verifying dependencies...
===> Upgrading lager v3.6.8
===> Version cached at /home/pete/.cache/rebar3/hex/hexpm/packages/lager-3.6.8.tar is up to date, reusing it
===> No upgrade needed for amqp_client
===> No upgrade needed for jsx
===> Upgrading lager v3.6.9
===> Version cached at /home/pete/.cache/rebar3/hex/hexpm/packages/lager-3.6.9.tar is up to date, reusing it

In short:

===> Fetching bbmustache ({pkg,<<"bbmustache">>,<<"1.6.0">>})
===> Fetching bbmustache v1.6.0

@fenollp fenollp closed this Mar 26, 2019
@fenollp fenollp reopened this Mar 26, 2019
@fenollp
Copy link
Contributor Author

fenollp commented Mar 26, 2019

(misclick)
Any help on making CI pass? The tests are a bit terse.

@ferd
Copy link
Collaborator

ferd commented Mar 26, 2019

I believe that the tests for all the failing suites are parsing the warnings to make sure they are properly output. See for example:

  • warning_calls() ->
    History = meck:history(rebar_log),
    [{Str, Args} || {_, {rebar_log, log, [warn, Str, Args]}, _} <- History].
    error_calls() ->
    History = meck:history(rebar_log),
    [{Str, Args} || {_, {rebar_log, log, [error, Str, Args]}, _} <- History].
    check_warnings(_, [], _) ->
    ok;
    check_warnings(Warns, [{Type, Name, Vsn} | Rest], mixed) ->
    ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]),
    ?assert(in_warnings(Type, Warns, Name, Vsn)),
    check_warnings(Warns, Rest, mixed);
    check_warnings(Warns, [{Name, Vsn} | Rest], Type) ->
    ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]),
    ?assert(in_warnings(Type, Warns, Name, Vsn)),
    check_warnings(Warns, Rest, Type);
    check_warnings(Warns, none, _Type) ->
    ct:pal("Checking that there were no warnings", []),
    ?assert(Warns == []).
    in_warnings(git, Warns, NameRaw, VsnRaw) ->
    Name = iolist_to_binary(NameRaw),
    1 =< length([1 || {_, [AppName, {git, _, {_, Vsn}}]} <- Warns,
    AppName =:= Name, Vsn =:= VsnRaw]);
    in_warnings(pkg, Warns, NameRaw, VsnRaw) ->
    Name = iolist_to_binary(NameRaw),
    Vsn = iolist_to_binary(VsnRaw),
    1 =< length([1 || {_, [AppName, {pkg, _, AppVsn}]} <- Warns,
    AppName =:= Name, AppVsn =:= Vsn]).
  • 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), ?config(deps_type, 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], Type) ->
    ct:pal("Checking for warning ~p in ~p", [{Name,Vsn},Warns]),
    ?assert(in_warnings(Type, Warns, Name, Vsn)),
    check_warnings(Warns, Rest, Type).
    in_warnings(git, Warns, NameRaw, VsnRaw) ->
    Name = iolist_to_binary(NameRaw),
    1 =< length([1 || {_, [AppName, {git, _, {_, Vsn}}]} <- Warns,
    AppName =:= Name, Vsn =:= VsnRaw]);
    in_warnings(pkg, Warns, NameRaw, VsnRaw) ->
    Name = iolist_to_binary(NameRaw),
    Vsn = iolist_to_binary(VsnRaw),
    1 =< length([1 || {_, [AppName, {pkg, _, AppVsn}]} <- Warns,
    AppName =:= Name, AppVsn =:= Vsn]).

These are two suites that look for warnings (there may be more, but I think that's the largest bit).
By changing the output of the builds, you inadvertently broke the tests that relied on that format (as some external tools might have)

It's a question of updating the tests to care for the new warnings, and then possibly having a decent time where master contains the fix but without releasing to make sure anyone running tools find the problem early enough to not break their stuff. At least git resources will remain the same so that limits some potential breakage too.

Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
Signed-off-by: Pierre Fenoll <pierrefenoll@gmail.com>
@fenollp
Copy link
Contributor Author

fenollp commented May 6, 2019

I can squash commits. Also maybe you'd prefer me find another way of making these tests pass?

@ferd
Copy link
Collaborator

ferd commented May 6, 2019

The windows test appears to always be broken on CI, we might just want to remove it (CC @tsloughter ) -- I'll run this branch by hand instead on my windows computer

@tsloughter
Copy link
Collaborator

Ugh, yea, I guess we should.

@ferd ferd merged commit 4b610f5 into erlang:master May 6, 2019
@fenollp fenollp deleted the p-hex branch May 7, 2019 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants