erlang / rebar3

Erlang build tool that makes it easy to compile and test Erlang applications and releases.
http://www.rebar3.org
Apache License 2.0
1.71k stars 517 forks source link

rebar ct loads compile modules #1958

Closed RoadRunnr closed 6 years ago

RoadRunnr commented 6 years ago

With rebar 3.7 my common tests suddenly started to fail with an error {already_loaded,module_under_test}. Turns out that before 3.7, rebar3 did not load the applications and their modules and my test suite contains a ok = application:load(module_under_test).

Please restore the pre 3.7 behavior of not preloading the application and modules under test.

Tested with 3.6.1 (works) and 3.7.4 (broken) on OTP 21.0.9.

ferd commented 6 years ago

The loading of applications and modules was part of a large fix for code path handling to allow plugins and the compiler to work better.

The problem being that some applications, such as say, bbmustache can be needed both by an application and rebar3 itself (for templates); there are also other libraries like erlware_commons that could fit that bill.

To fix the path loading and handling, the application is unloaded and reloaded, the list of modules fetched, and the module paths checked against what is the expected path of the application. This is done, among other things, to ensure that your tests actually run against the right versions of applications being used. Prior to this, multiple edge cases existed for plugins and applications where the wrong version would sometimes be loaded or not loaded at all when it should have been, and could cause full crashes.

We could re-unload the applications that were not loaded after the fact, but it appears that this would risk a ~10% performance penalty for rebar3 on almost any task run. I'll check into doing that, but we'll see if this generates other complaints instead.

ferd commented 6 years ago

Hm, looking at https://github.com/erlang/rebar3/blob/99a7b7ea36bea41b687475ccfd4dee7dfd6d36d3/src/rebar_paths.erl#L101-L113 it appears we only reload applications that were already loaded, so this cannot be the culprit right here. See for example, trying to unload an application that was not loaded:

1> application:unload(observer).
{error,{not_loaded,observer}}

Which means that the load call is conditional to the app already having been loaded before.

The Common Test handler for its part only reloads the application if it is in sys.config:

https://github.com/erlang/rebar3/blob/99a7b7ea36bea41b687475ccfd4dee7dfd6d36d3/src/rebar_prv_common_test.erl#L253-L255

And this last reloading has been around for 2 years.

Here's a list of all the annotated calls we make:


%% used only when debugging rebar3 itself (`rebar3 report <command>`)
src/rebar_prv_report.erl
46:    [application:load(App) || App <- Apps],

%% the one I checked above
src/rebar_paths.erl
107:             ok -> application:load(AtomApp);

%% loading the tool itself
src/rebar3.erl
365:    _ = application:load(rebar),

%% loading dialyzer as needed for its task
src/rebar_prv_dialyzer.erl
567:    _ = application:load(dialyzer),

%% those are called in some older path handling functions kept for backwards compat
%% and some tasks and have been around for 3 years
src/rebar_utils.erl
779:                          application:load(App),
804:                          application:load(App),

%% used as part of loading and starting applications according to sys.config and relx config
src/rebar_prv_shell.erl
350:    [case application:load(App) of
357:            not lists:keymember(App, 1, application:loaded_applications())],

%% called as part of path-refreshing in the shell after recompiling an app
src/rebar_agent.erl
192:            application:load(App),

%% used as part of config loading as above
src/rebar_prv_common_test.erl
254:    [application:load(Application) || Config <- Configs, {Application, _} <- Config],

%% load a plugin to run it
src/rebar_plugins.erl
154:    _ = application:load(Plugin),

There is no other call to application:load in rebar3, and otherwise rebar3 starts its own dependencies.

What's the app name of the component for which this fails?

I'm wondering if your app name could have been one caught in cross-dependencies between either Rebar3's own deps, or with current plugins you might have. This would explain the change in behaviour. Otherwise I think it should have been stable.

RoadRunnr commented 6 years ago

Hi Fred,

Fred Hebert notifications@github.com schrieb am Mo., 26. Nov. 2018 um 15:43 Uhr:

[...]

What's the app name of the component for which this fails?

I'm wondering if your app name could have been one caught in cross-dependencies between either Rebar3's own deps, or with current plugins you might have. This would explain the change in behaviour. Otherwise I think it should have been stable.

The name is ergw_aaa, so this shouldn't collide.

I have create a work branch to demonstrate the failure at https://github.com/travelping/ergw_aaa/tree/work/rebar. If you run ct with rebar3 ct --suite=diameter_SUITE, rebar3 3.6.1 will work, 3.7.4 fails.

Andreas

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/erlang/rebar3/issues/1958#issuecomment-441662360, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUkMbBE7MGyIclyxyDzG1Zht2LBxg4pks5uy_4egaJpZM4YyuhY .

-- -- Dipl.-Inform. Andreas Schultz

----------------------- enabling your networks ---------------------- Travelping GmbH Phone: +49-391-81 90 99 0 Roentgenstr. 13 Fax: +49-391-81 90 99 299 39108 Magdeburg Email: info@travelping.com GERMANY Web: http://www.travelping.com

Company Registration: Amtsgericht Stendal Reg No.: HRB 10578 Geschaeftsfuehrer: Holger Winkelmann VAT ID No.: DE236673780

ferd commented 6 years ago
===> Running Common Test suites...
%%% config_SUITE: .
%%% diameter_Gx_SUITE: ..
%%% diameter_Gy_SUITE: ..
%%% diameter_Rf_SUITE: .
%%% diameter_SUITE: ....
All 10 tests passed.

I couldn't get the failure you mention with a regular ct call, but I can reproduce with your specific call:

λ ergw_aaa → work/rebar → rebar3 ct --suite test/diameter_SUITE
===> Verifying dependencies...
===> Compiling ergw_aaa
===> Compiling diameter files...
===> Running Common Test suites...
%%% diameter_SUITE:
%%% diameter_SUITE ==> init_per_suite: FAILED
%%% diameter_SUITE ==> {{badmatch,{error,{already_loaded,ergw_aaa}}},
 [{diameter_SUITE,init_per_suite,1,
                  [{file,"/private/tmp/ergw_aaa/test/diameter_SUITE.erl"},
                   {line,82}]},
  {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1545}]},
  {test_server,run_test_case_eval1,6,[{file,"test_server.erl"},{line,1148}]},
  {test_server,run_test_case_eval,9,[{file,"test_server.erl"},{line,995}]}]}

So I checked a bit and it turns out that the issue is related to cover compilation; that's the one component that force-loaded some applications because it was left on the old path handling routine.

The problem was that the old path handling routine did manage to force loading a bunch of applications, but it did not necessarily unload them aside from specific calls that the common test handler used to need to do, but no longer needed (or so I thought) under the updated path management.

The leftover calls in the cover compilation bits caused the app to be loaded and to remain loaded. I'm running a few tests right now, but switching the cover compilation to the new path management seems to fix that problem. I'll probably have a patch ready during my lunch break, though I think this might be minor enough not to warrant a patch release right away.

ferd commented 6 years ago

Please try this PR: https://github.com/erlang/rebar3/pull/1959