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

Allow for less verbose test execution output on SKIPPED #2821

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

Conversation

paulo-ferraz-oliveira
Copy link
Contributor

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Aug 17, 2023

The change

As proposed by ferd/cth_readable#41, instead of SKIPPED we get a magenta *, if verbose is true.

Vendoring cth_readable in

I vendored cth_readable 1.6.0 in by changing apps/rebar/rebar.config to use the new version, then ran rebar3 experimental vendor from the root folder (after removing the vendor and vendor_plugins folders).

Not all of the vendored changes are from my previous pull request, and part of the result of command vendor (that which presented changes outside the cth_readable folder) were omitted :erlware_commons' rebar.config.script is also updated - without changing the reference version.

Default verbose

The default verbose is false; I wonder if the output of the tests (from e.g. stdout) is considered part of the interface.

Example output after the change

Ex. (as per the current integration):

===> Running Common Test suites...
%%% ex1_SUITE: *
Skipped 1 (1, 0) tests. Passed 0 tests.

Closes #2809.

Instead of SKIPPED we get a magenta *..., if verbose is true
@paulo-ferraz-oliveira
Copy link
Contributor Author

@ferd, did you eventually get around to this one? Cheers.

@ferd
Copy link
Collaborator

ferd commented Aug 25, 2023

not yet, hectic work week and busy nights, I had no bandwidth for OSS stuff this week.

@paulo-ferraz-oliveira
Copy link
Contributor Author

Sure, no hurry, I understand how it goes. Was just gently bumping 😄

@ferd
Copy link
Collaborator

ferd commented Aug 26, 2023

hm, not seeing it apply well.

λ [vps] rebar3 → paulo-ferraz-oliveira-feature/less-verbose-output-for-skipped → rebar3 ct
....
%%% rebar_compile_SUITE: ...................................................................................
%%% rebar_compile_SUITE ==> always_recompile_when_erl_compiler_options_set: SKIPPED
%%% rebar_compile_SUITE ==> {tc_user_skip,"compile:env_compiler_options/0 available"}
.....
λ [vps] rebar3 → paulo-ferraz-oliveira-feature/less-verbose-output-for-skipped → rebar3 ct --verbose=true
...
CWD set to: "/home/ferd/code/self/rebar3/_build/test/logs/ct_run.nonode@nohost.2023-08-26_00.28.13"

TEST INFO: 2 test(s), 627 case(s) in 40 suite(s)

Testing lib.certifi: Starting test, 0 test cases
Testing lib.certifi: TEST COMPLETE, 0 ok, 0 failed of 0 test cases

Testing lib.rebar: Starting test, 627 test cases
...
%%% rebar_compile_SUITE: ...................................................................................Testing lib.rebar: *** SKIPPED test case 103 of 627 ***

%%% rebar_compile_SUITE ==> always_recompile_when_erl_compiler_options_set: SKIPPED
%%% rebar_compile_SUITE ==> {tc_user_skip,"compile:env_compiler_options/0 available"}
.....
λ [vps] rebar3 → paulo-ferraz-oliveira-feature/less-verbose-output-for-skipped → rebar3 ct --verbose=false
...
%%% rebar_compile_SUITE: ...................................................................................
%%% rebar_compile_SUITE ==> always_recompile_when_erl_compiler_options_set: SKIPPED
%%% rebar_compile_SUITE ==> {tc_user_skip,"compile:env_compiler_options/0 available"}
.....

Or is it only being silent on specific skip types?

@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Aug 26, 2023

I didn't mean to make it specific, so it's possible something's off. Is this running on rebar3 itself after bootstrap?

Edit: it's possible I'm only picking up stuff from rebar.config via ct_opts. I'll check how the CLI's input relates to that one.

The CLI, however, takes precedence from the .config file
@paulo-ferraz-oliveira paulo-ferraz-oliveira force-pushed the feature/less-verbose-output-for-skipped branch from 0631798 to fd487a9 Compare September 15, 2023 22:36
@paulo-ferraz-oliveira
Copy link
Contributor Author

Pushed fd487a9 with a minor change in implementation: the CLI takes precedence over the .config.

Results

Results are as follows.

Below:

  • CLI is equivalent to --verbose=true/false
    • (or undefined if not present)
  • config is equivalent {ct_opts, [{verbose, true/false}]}.
    • (or undefined if not present)

For:

  • CLI undefined / config undefined
  • CLI undefined / config false
  • CLI false / config undefined
  • CLI false / config false
  • CLI false / config true

we get

%%% rebar3_test_SUITE: *******************

For:

  • CLI undefined, config true
  • CLI true / config undefined
  • CLI true / config false
  • CLI true / config true

we get

%%% rebar3_test_SUITE ==> test1: SKIPPED
%%% rebar3_test_SUITE ==> {tc_user_skip,"skipped"}
Testing lib.rebar3_test: *** SKIPPED test case 2 of 2 ***

%%% rebar3_test_SUITE ==> test2: SKIPPED
%%% rebar3_test_SUITE ==> {tc_user_skip,"skipped"}
Testing lib.rebar3_test: *** SKIPPED test case 2 of 2 ***

The default behavior is verbose=false, in the most recent commit.

Copy link
Collaborator

@ferd ferd left a comment

Choose a reason for hiding this comment

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

I think this now works okay and can be merged, though we may have a subtle bug in the library when a whole suite gets skipped:

===> Running Common Test suites...
%%% cfg_SUITE: ...
%%% session_SUITE: ....
%%% dirmon_event_SUITE: ..
%%% dirmon_tracker_SUITE: ...................
%%% revault_fsm_SUITE: ...............................
%%% revault_tls_SUITE: .....
%%% s3_integration_SUITE: ************%%% boot_SUITE: .
Skipped 10 (10, 0) tests. Passed 65 tests.

we get a missing linebreak on the trailing end of the skipped element if the next one is expected to be a final point.

I think this may need to be fixed at the cth_readable level though, tricky one to see.

I don't think it should be a blocker, but I also don't know if that might mess up tools that parse rebar3 suite output

@paulo-ferraz-oliveira
Copy link
Contributor Author

I think this may need to be fixed at the cth_readable level though, tricky one to see.

I can look at this when I have some time. Otherwise I can also try to hack something in rebar3 now and fix cth_readable later. I need to look at it deeper. In any case, thanks for taking the time to review.

@ferd
Copy link
Collaborator

ferd commented Sep 25, 2023

yeah no problem. An option is to at least do the patching in the vendored code so we can test it more easily, I don't know if I'll have programming time set aside for OSS stuff this week yet.

@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Oct 14, 2023

I can't seem to replicate this, though I'll keep trying.

Here's an example output

%%% a_SUITE: **
%%% b_SUITE: ..
%%% c_SUITE: **
Skipped 4 (4, 0) tests. Passed 2 tests.

The whole a_SUITE is skipped...

Ah, this might depend on "readable" vs. "compact" shell. Lemme look closer.

Edit: I don't know what options you're using. 😄

@paulo-ferraz-oliveira
Copy link
Contributor Author

Something seems kinda broken already with rebar3 (below is execution for 3.22.1):

%%% rebar3_test_SUITE: .%%% rebar3_test_SUITE ==> files_empty_ok: OK
.%%% rebar3_test_SUITE ==> files_fish_nok: OK
.%%% rebar3_test_SUITE ==> opt_color_auto: OK
.%%% rebar3_test_SUITE ==> opt_check_sourced: OK
.%%% rebar3_test_SUITE ==> opt_include: OK

(this is with option {ct_opts, [{ct_hooks, [cth_readable_shell]}]}.)

@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Oct 14, 2023

I've gotten to a point (with some control flags) where stuff in the compact shell looks like

% (`verbose=true`)

Testing extras.mylib: Starting test, 8 test cases
%%% a_SUITE: Testing extras.mylib: *** SKIPPED test case 1 of 8 ***
%%% a_SUITE ==> test1: SKIPPED
%%% a_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% a_SUITE: Testing extras.mylib: *** SKIPPED test case 2 of 8 ***
%%% a_SUITE ==> test2: SKIPPED
%%% a_SUITE ==> {tc_user_skip,"SKIPPED"}

%%% b_SUITE: ..

%%% c_SUITE: Testing extras.mylib: *** SKIPPED test case 5 of 8 ***
%%% c_SUITE ==> test1: SKIPPED
%%% c_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% c_SUITE: Testing extras.mylib: *** SKIPPED test case 6 of 8 ***
%%% c_SUITE ==> test2: SKIPPED
%%% c_SUITE ==> {tc_user_skip,"SKIPPED"}

%%% d_SUITE: ..
Testing extras.mylib: TEST COMPLETE, 4 ok, 0 failed, 4 skipped of 8 test cases

and

% (`verbose=false`)

%%% a_SUITE: **
%%% b_SUITE: ..
%%% c_SUITE: **
%%% d_SUITE: ..
Skipped 4 (4, 0) tests. Passed 4 tests.

but still I'd like to confirm you rebar.config options to understand how you got to the results here in the first place.

@ferd
Copy link
Collaborator

ferd commented Oct 23, 2023

Sorry, just getting back to this this week. I was using the default Rebar3 file in the rebar3 project with the commands rebar3 ct --verbose=true and rebar3 ct --verbose=false. I may need to try it on more OSes if it's not reproducible.

Via a couple of control flags (assuming sequential
test execution) we're able to output better visuals

We make use of HMod:post_end_per_testcase/5 as supported
by most recent versions of Erlang 21+

This is not yet a complete change since:
1. we're doing it for demo purposes
2. we're not replicating the changes to cth_readable_shell
@paulo-ferraz-oliveira paulo-ferraz-oliveira force-pushed the feature/less-verbose-output-for-skipped branch from 80c2238 to e8428ac Compare March 6, 2024 00:33
Code was mostly redone as manipulation of ~n and related
elements didn't suit the current requirements
@paulo-ferraz-oliveira paulo-ferraz-oliveira force-pushed the feature/less-verbose-output-for-skipped branch from e8428ac to fa35b88 Compare March 6, 2024 04:11
@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Mar 6, 2024

@ferd, I'm revisiting this, but am not in a hurry to get it reviewed. I just had it in my TODO and wanted to not forget it.

Current results follow. I added more variety to tests, started covering "all skipped" suites and re-worked the cth_readable code too. If all goes well, and this gets accepted, I'll first pull request to cth_readable again, see that released and then import here.

  • verbose == true
%%% a_SUITE: .
%%% a_SUITE ==> Testing extras.mylib: *** SKIPPED test case 2 of 15 ***
%%% a_SUITE ==> test2: SKIPPED
%%% a_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% a_SUITE: .
%%% b_SUITE ==> Testing extras.mylib: *** SKIPPED test case 4 of 15 ***
%%% b_SUITE ==> test1: SKIPPED
%%% b_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% b_SUITE: .
%%% b_SUITE ==> Testing extras.mylib: *** SKIPPED test case 6 of 15 ***
%%% b_SUITE ==> test3: SKIPPED
%%% b_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% c_SUITE ==> Testing extras.mylib: *** SKIPPED test case 7 of 15 ***
%%% c_SUITE ==> test1: SKIPPED
%%% c_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% c_SUITE ==> Testing extras.mylib: *** SKIPPED test case 8 of 15 ***
%%% c_SUITE ==> test2: SKIPPED
%%% c_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% c_SUITE: .
%%% d_SUITE ==> Testing extras.mylib: *** SKIPPED {d_SUITE,init_per_suite} ***
%%% d_SUITE ==> init_per_suite: SKIPPED
%%% d_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% d_SUITE ==> test1: SKIPPED
%%% d_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% d_SUITE ==> test2: SKIPPED
%%% d_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% d_SUITE ==> test3: SKIPPED
%%% d_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% d_SUITE ==> end_per_suite: SKIPPED
%%% d_SUITE ==> {tc_user_skip,"SKIPPED"}
%%% e_SUITE: ..
%%% e_SUITE ==> Testing extras.mylib: *** SKIPPED test case 15 of 15 ***
%%% e_SUITE ==> test3: SKIPPED
%%% e_SUITE ==> {tc_user_skip,"SKIPPED"}
  • verbose == false
%%% a_SUITE: .*.
%%% b_SUITE: *.*
%%% c_SUITE: **.
%%% d_SUITE: *****
%%% e_SUITE: ..*

where . is success, * is skipped.

Edit: I just saw it's not finishing properly, since it's not outputting the very last \n. I'll get to this when I have more time 😄

... for last executed suite

And take this time to filter out end_per_suite and init_per_suite
from the test case output (when skipped)
@paulo-ferraz-oliveira
Copy link
Contributor Author

paulo-ferraz-oliveira commented Mar 6, 2024

Pushed a new fix. If this is Ok I can maybe move it to cth_readable, then use that vendored code here. Also, it seems both the compact and non-compact cth_readable shell are mighty similar, and it might've been unintentionally broken here - the readable shell should probably not care about verbosity options (?). It seems this change was never released, though.

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.

Reduce verbosity on skipped testcases' output
2 participants