Problem/Motivation

Simpletest and its UI is no longer used. However we still have a class at \Drupal\Tests\Listeners\SimpletestUiPrinter that turns URLs into clickable links.

PHPUnit 10 removes the —printer command line option: https://github.com/sebastianbergmann/phpunit/blob/master/ChangeLog-10.0.md

Steps to reproduce

Proposed resolution

Remove the class, it’s essentially dead code.

Remaining tasks

Figure out if we still need this.

User interface changes

API changes

Data model changes

Release notes snippet

Comments

longwave created an issue. See original summary.

longwave’s picture

mondrake’s picture

Issue tags: +PHPUnit 10
murilohp’s picture

Status: Active » Needs review
StatusFileSize
new600 bytes

I agree with @mondrake, the following patch removes the class.

If we need to remove any other class please let me know.

Thanks!

mondrake’s picture

I am afraid it won’t work… there is code that is using that class. BTW there’s zero test coverage of this, I wonder if we can have some.

mondrake’s picture

Assigned: Unassigned » mondrake

working on a test

Status: Needs review » Needs work

The last submitted patch, 4: 3254723-4.patch, failed testing. View results

mondrake’s picture

Assigned: mondrake » Unassigned
StatusFileSize
new1.41 KB

Here's a test that can be included in next patch to ensure we do not break anything.

mondrake’s picture

StatusFileSize
new1.43 KB

Sorry, please disregard the patch in #7.

mondrake’s picture

Status: Needs work » Needs review
StatusFileSize
new4.2 KB

Here's a full patch.

mondrake’s picture

StatusFileSize
new485 bytes
new4.37 KB
murilohp’s picture

Hey @mondrake, thanks a lot for your help! At the begging I thought it could be an easy fix, but after the tests of #4, I realized that this could be harder, I reviewed the patch #11 and it looks good to me, I'll not move this to RTBC right now because I think we should wait for someone else's review.

For me it's +1 for RTBC.

Thanks!

mondrake’s picture

Title: Remove or rename SimpletestUiPrinter » Remove SimpletestUiPrinter
mondrake’s picture

StatusFileSize
new3.82 KB
new1.81 KB

Couple glitches:

  • I think we do not need to override the write method
  • removed too verbose message in the test, the standard message will be enough
longwave’s picture

Status: Needs review » Reviewed & tested by the community

Looks great to me.

As a kind of next step I wonder if we could refactor HtmlOutputPrinterTrait into HtmlOutputPrinter, we don't really need a trait here - I think this was for cross-version compatibility at some point.

mondrake’s picture

Not sure it's worth at this point, in PHPUnit 10 all this has to be rewitten anyway.

mondrake’s picture

Issue summary: View changes

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 14: 3254723-14.patch, failed testing. View results

mondrake’s picture

Status: Needs work » Reviewed & tested by the community

random test failure

  • catch committed b5c3f1b on 10.0.x
    Issue #3254723 by mondrake, murilohp, longwave: Remove...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed b5c3f1b and pushed to 10.0.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.