Proposal for progress output when using teamcity.

[SakiTakamachi]

Overview

Changed to allow selection of output state when used in conjunction with the "--no-progress" option, rather than unconditionally turning off progress when "--teamcity" is used.

Why ?

It is common to use var_dump() or other debugging methods during testing.

The "--teamcity" option turns off the progress display and the associated debugging output.

I have seen people on social networking sites and around me having trouble with this, so I thought it would be a good idea to combine the "--no-pregress" option to allow users to choose their own behavior.

Other ways

Or, if you prefer, I can add an option such as "--teamcity-with-progress" to achieve the same thing, and I will modify it accordingly.

Test

Fixed test failures other than 1348 in the regression test, which had originally failed.
Added a test without "--no-progress" of teamcity and renamed tests with "--no-progress".

[sebastianbergmann]

If JetBrains / the PhpStorm team is fine with this change then I will accept this (after the tests that now fail due to changed output have been updated).

[SakiTakamachi]

Thank you very much.
First of all, I have corrected the test.
I await the conclusion of JetBrains / the PhpStorm team.

[codecov]

Codecov Report

Merging #5367 (1f28df3) into main (f85cc67) will decrease coverage by 0.65%.
The diff coverage is n/a.

❗ Current head 1f28df3 differs from pull request most recent head d33e579. Consider uploading reports for the commit d33e579 to get more accurate results

@@             Coverage Diff              @@
##               main    #5367      +/-   ##
============================================
- Coverage     85.67%   85.02%   -0.65%     
+ Complexity     6181     6126      -55     
============================================
  Files           654      654              
  Lines         19668    19446     -222     
============================================
- Hits          16850    16534     -316     
- Misses         2818     2912      +94     

Impacted Files	Coverage Δ
src/TextUI/Output/Facade.php	97.50% <ø> (-0.05%)	⬇️

... and 149 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[SakiTakamachi]

Do I need to add new test cases?

Currently, the only test case is a combination of "--teamcity" and "--no-progress" and there is no test case for "--teamcity" only.

PS: added.

[SakiTakamachi]

@sebastianbergmann

Is there anything I should do about this pull request?

If there is nothing in particular I should do, I will wait as I know you are all busy with other tasks!

[SakiTakamachi]

Fixed conflicts and aligned new tests with upstream specs.

[SakiTakamachi]

@isfedorov
I found a member of JetBrains and would like to hear your opinion.
Please excuse my abruptness.

What do you think about this PR?

[isfedorov]

@SakiTakamachi Could you please clarify why do you need to run PhpUnit tests from CLI with flag --teamcity ? This flag has a dedicated purpose to be used only internally in PhpStorm

[SakiTakamachi]

@isfedorov
Thank you for your confirmation!

The goal is to control the display of progress when running tests in PhpStorm, rather than executing them from the CLI.

[isfedorov]

@SakiTakamachi Thank you, got it. Could you please clarify why you need to display progress in output rather than just use PhpStorm's indicators in the related tool window? The tool window can already display in progress, failed, passed skipped etc tests with dedicated icons.

What benefits will such additional data in output bring to users?

[SakiTakamachi]

@isfedorov
Thank you for the reference materials.

The intention is to facilitate debugging during test execution by using functions like var_dump(), aiming to output debug data rather than desiring progress display.

[SakiTakamachi]

Currently, the control of output for debugging is associated with the control of progress display.

[isfedorov]

@SakiTakamachi Thank you for the clarification.

Currently, the control of output for debugging is associated with the control of progress display.

That sounds a bit strange to me. It looks to me that the control of progress (--no-progress) should control only displaying of progress (like dots in CLI ..S..F..I etc) and a separate dedicated option for additional verbose/debugging (f.ex. --verbose/--debug) should control additional debug. @sebastianbergmann Can you comment on this?

For sure PhpStorm can pass by default --no-progress to avoid displaying F/S/I/etc state of tests in output by default for everyone, but still it looks strange that the option named --no-progress controls additional debug data

[sebastianbergmann]

PHPUnit uses PHP's output buffering to capture output made by test(ed) code. When the test does not perform assertions on captured output then the output is considered unexpected and an PrintedUnexpectedOutput event is emitted. The ProgressPrinter (which is controlled by the --no-progress CLI option) is currently not only responsible for printing progress (., E, F, ...) but also for printing unexpected output.

[sebastianbergmann]

Thank you for your contribution. I appreciate the time you invested in preparing this pull request. However, I have decided not to merge it, see #5445.