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

Make PHPUnit\TextUI\ResultPrinter an interface #4024

Closed
jaydiablo opened this issue Jan 21, 2020 · 2 comments
Closed

Make PHPUnit\TextUI\ResultPrinter an interface #4024

jaydiablo opened this issue Jan 21, 2020 · 2 comments
Labels
type/backward-compatibility Something will be/is intentionally broken type/refactoring A refactoring that should be applied to make the code easier to understand and maintain
Milestone

Comments

@jaydiablo
Copy link

Q A
PHPUnit version 8.x+ (8.5.2 for sure)
PHP version any
Installation Method n/a

Summary

The PHPUnit\TextUI\ResultPrinter class is marked as internal (https://github.com/sebastianbergmann/phpunit/blob/master/src/TextUI/ResultPrinter.php#L29) which is causing notices to be emitted during test runs that have an external result printer configured:

diablomedia/phpunit-pretty-printer#27
mikeerickson/phpunit-pretty-result-printer#155
indentno/phpunit-pretty-print#28

/var/www/html $ bin/phpunit --filter ArraysTest
PHPUnit 8.5.2 by Sebastian Bergmann and contributors.

Testing Project Test Suite

                       App\Tests\Common\ArraysTest ...

Time: 216 ms, Memory: 8.00 MB

OK (3 tests, 6 assertions)

Other deprecation notices (1)

  1x: The "PHPUnit\TextUI\ResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "DiabloMedia\PHPUnit\Printer\PrettyPrinter".

Is it possible to have this class be part of the public API as mentioned in #3236 ?

If not, what is the recommended way to implement result printers?

Thanks!

@jaydiablo jaydiablo added the type/bug Something is broken label Jan 21, 2020
@sebastianbergmann
Copy link
Owner

Let me start by saying that

Other deprecation notices (1)

  1x: The "PHPUnit\TextUI\ResultPrinter" class is considered internal This class is not covered by the backward compatibility promise for PHPUnit. It may change without further notice. You should not use it from "DiabloMedia\PHPUnit\Printer\PrettyPrinter".

is not emitted by PHPUnit. I assume that you are using a wrapper around PHPUnit, probably something like Symfony's PHPUnit bridge.

With that out of the way, yes, the fact that PHPUnit\TextUI\ResultPrinter is marked @internal is a problem. Its implementation is, and should stay, internal. However, for legacy reasons, it is also a public API as extending this class is an intended way for augmenting or changing the output printed by PHPUnit.

I think the best way forward is to

  • Rename PHPUnit\TextUI\ResultPrinter to PHPUnit\TextUI\DefaultResultPrinter and keep it marked as @internal
  • Make PHPUnit\TextUI\ResultPrinter an interface implemented by PHPUnit\TextUI\DefaultResultPrinter; this interface would not be marked @internal

This change will break backward compatibility, of course.

@sebastianbergmann sebastianbergmann added type/backward-compatibility Something will be/is intentionally broken type/refactoring A refactoring that should be applied to make the code easier to understand and maintain and removed type/bug Something is broken labels Jan 22, 2020
@sebastianbergmann sebastianbergmann added this to the PHPUnit 9.0 milestone Jan 22, 2020
@sebastianbergmann sebastianbergmann changed the title Can the PHPUnit\TextUI\ResultPrinter class be considered public API? Make PHPUnit\TextUI\ResultPrinter an interface Jan 22, 2020
@epdenouden
Copy link
Contributor

@jaydiablo The logging code could use a good spring cleaning, however this will have to wait until the replacement for TestListeners is ready for production.

The current listener system is hard to work with with newer use cases like the execution reordering. If you want to get some feel for the hoops to jump through, have a look at #3417.

I assume that you are using a wrapper around PHPUnit, probably something like Symfony's PHPUnit bridge.

Yes, this is one of the functionalities that the bridge offers which needs to stay after the refactoring.

JeppeKnockaert added a commit to JeppeKnockaert/phpunit-documentation-english that referenced this issue Mar 20, 2020
This was changed in version 9.0 with the implemtation of sebastianbergmann/phpunit#4024.
sebastianbergmann pushed a commit that referenced this issue Jul 29, 2020
Update the default printerClass to DefaultResultPrinter.

This matches the change in #4024
steve-todorov added a commit to steve-todorov/phpunit-detailed-printer that referenced this issue Aug 10, 2020
steve-todorov added a commit to steve-todorov/phpunit-detailed-printer that referenced this issue Aug 10, 2020
rudowastaken pushed a commit to LimeDeck/phpunit-detailed-printer that referenced this issue Aug 10, 2020
* issue-21: Updating PHPUnit dependency to 9

* issue-21: Fixed phpunit configuration warning with phpunit --migrate-configuration.

* issue-21: Switching from ResultPrinter to DefaultResultPrinter

Check sebastianbergmann/phpunit#4024

* issue-21: Fixed failing test case - output no longer includes time and memory for this case.

Co-authored-by: Jakub Homoly <jakubhomoly@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/backward-compatibility Something will be/is intentionally broken type/refactoring A refactoring that should be applied to make the code easier to understand and maintain
Projects
None yet
Development

No branches or pull requests

3 participants