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.

Sign up for GitHub

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

Clean up no longer present test suites from _build #2626

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Clean up no longer present test suites from _build #2626

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d685a78
Cleanup no longer present test suites from _build
4tyTwo Oct 9, 2021
21a2ce0
Handle case when there are no test sources
4tyTwo Oct 10, 2021
356c0aa
Delete unreachable clause in rebar_prv_common_test:do/2
4tyTwo Oct 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 47 additions & 14 deletions src/rebar_prv_common_test.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,12 @@ do(State) ->
case compile(State, Tests) of
%% successfully compiled apps
{ok, S} ->
{ok, T} = Tests,
TestSources = proplists:get_value(dir, T, []),
AllDeps = rebar_state:code_paths(S, all_deps),
IsTestDir = fun(Path) -> string:slice(Path, length(Path) - 4, 4) == "test" end,
Copy link
Collaborator

@ferd ferd Oct 25, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test you're looking here though might be the profile of the path, which would not be accurate in all cases. For example, you might get test+demo if you ran rebar3 as demo ct

We don't run tests on deps, just on top-level apps, which can be found with rebar_state:project_apps(State) (which you can then expand with rebar_app_info:out_dir/1 to get the app's path in _build), so that should cut down the search space.

Additionally, in umbrella applications where a top-level test/ directory exists, the path should be _build/<profile>/extras/test/.

The last final weird case is visible in rebar3. We have some integration suites that run only on Linux that can be called by running rebar3 as systest ct. The directories can be seen in:

→ ls _build/systest+test/lib/rebar/
ebin  include  priv  src  systest  test

where test cases exist both in systest/ and in test/.

Common test integration is really messy, and my understanding/idea is that we should be able to take the {dir, ...} entries and plug them onto the out_dir(AppInfo) result to get all the entries, plus the extras path if it exists.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my understanding/idea is that we should be able to take the {dir, ...} entries and plug them onto the out_dir(AppInfo)

While I can see it working, I can't think of an actually nice way of doing it. I'll try giving it another thought again

This last case when it is possible to provide arbitrary directory which will contain test suites is kind of tricky to work around. My immediate solution would be just looking for .*_SUITE.erl files recursively in both extras and _build/PROFILE/lib/APP (excluding ebin, src, priv and include I guess).
Can you spot any obvious issues with that? My concern is that I'm not sure if any not test .erl files can exist outside the aforementioned 4 directories and therefore be in danger of being affected

Copy link
Collaborator

@ferd ferd Nov 6, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, anything that specifies paths in src_dirs or extra_src_dirs can contain valid Erlang files. It is configurable for each project.

CompiledTestsDirs = lists:filter(IsTestDir, AllDeps),
cleanup_unused_test_files(CompiledTestsDirs, TestSources),
{RawOpts, _} = rebar_state:command_parsed_args(S),
case proplists:get_value(compile_only, RawOpts, false) of
true ->
Expand All @@ -67,25 +73,52 @@ do(State, Tests) ->
%% Run ct provider pre hooks for all project apps and top level project hooks
rebar_hooks:run_project_and_app_hooks(Cwd, pre, ?PROVIDER, Providers, State),

case Tests of
{ok, T} ->
case run_tests(State, T) of
ok ->
%% Run ct provider post hooks for all project apps and top level project hooks
rebar_hooks:run_project_and_app_hooks(Cwd, post, ?PROVIDER, Providers, State),
rebar_paths:set_paths([plugins, deps], State),
symlink_to_last_ct_logs(State, T),
{ok, State};
Error ->
rebar_paths:set_paths([plugins, deps], State),
symlink_to_last_ct_logs(State, T),
Error
end;
{ok, T} = Tests,
case run_tests(State, T) of
ok ->
%% Run ct provider post hooks for all project apps and top level project hooks
rebar_hooks:run_project_and_app_hooks(Cwd, post, ?PROVIDER, Providers, State),
rebar_paths:set_paths([plugins, deps], State),
symlink_to_last_ct_logs(State, T),
{ok, State};
Error ->
rebar_paths:set_paths([plugins, deps], State),
symlink_to_last_ct_logs(State, T),
Error
end.

cleanup_unused_test_files(CompiledTestsDirs, TestSources) ->
GetModulesFun = fun(Dir, Acc) ->
Suites = rebar_utils:find_files(Dir, ".*_SUITE\.erl\$", false),
Modules = sets:from_list(lists:map(fun rebar_utils:erl_to_mod/1, Suites)),
sets:union(Acc, Modules)
end,
PresentTestModules = lists:foldl(GetModulesFun, sets:new(), TestSources),
CleanupFun = fun(Dir) ->
cleanup_unused_test_files_dir(Dir, PresentTestModules)
end,
lists:foreach(CleanupFun, CompiledTestsDirs).

cleanup_unused_test_files_dir(Dir, PresentTestModules) ->
Mapping = create_module_to_files_mapping(Dir),
RemoveFun = fun(Module, {SrcPath, BeamPath}) ->
case sets:is_element(Module, PresentTestModules) of
true -> ok;
false -> file:delete(SrcPath), file:delete(BeamPath)
end
end,
_ = maps:map(RemoveFun, Mapping),
ok.

create_module_to_files_mapping(Dir) ->
SrcFiles = rebar_utils:find_files(Dir, ".*_SUITE\.erl\$", false),
MappingFun = fun(SrcFile, Acc) ->
Module = rebar_utils:erl_to_mod(SrcFile),
BeamFile = filename:join([Dir, Module]) ++ ".beam",
Acc#{Module => {SrcFile, BeamFile}}
end,
lists:foldl(MappingFun, #{}, SrcFiles).

run_tests(State, Opts) ->
T = translate_paths(State, Opts),
Opts1 = setup_logdir(State, T),
Expand Down