"Buffered" TestDox printer(s)

[sebastianbergmann]

Thanks to #3092, #3147, and #3284, the order in which tests are run is no longer (always) based on "order in which DirectoryIterator finds *Test.php files" and "order of test methods within test class". This renders the TestDox result printer(s)'(s) output useless.

The TestDox result printer(s) should be changed to buffer test results in order to ensure the right order and then emit all output after the last test was run.

[sebastianbergmann]

@rpkamp @epdenouden What do you think?

[rpkamp]

It makes sense, although I think it would make sense if this were opt-in behaviour for the printer that could be enabled through some flag (like $expectNonLinearOutput) so that the default behaviour is as it is now (which is very handy I think) but can be switched to buffer when the normal flow doesn't make much sense.

[sebastianbergmann]

Sorry for not making this clear: yes, the buffering should be optional. The test runner can detect whether the option is required or not and enable it automatically when it is.

[epdenouden]

@sebastianbergmann I'm all for it when it is optional. I regularly use the visibility of the changed execution order of the TestDox printer to debug the sorters, for example in #3246 :-)

[epdenouden]

@sebastianbergmann Question: being as busy as you are, shall I pick up implementing this? I have some familiarity with this part of the codebase and I use the feature in my daily work. Nevermind if you're already working on it.

[sebastianbergmann]

@epdenouden That would be great -- and much appreciated. I cannot get to this before next week at the earliest.

[rpkamp]

@epdenouden Seeing how the TestListener will be deprecated it would make more sense to implement this as an extension I think. But that would be easier once the CliTestDoxPrinter is also an extension, as you could then just decorate it, making it more generic in case other extensions need it as well.

That does need some more thought on how to get the printers from TestListeners to extensions though.

[sebastianbergmann]

Not sure that printers should be extensions, but they should definitely use the TestHook interfaces instead of the TestListener interface. #3390 is about finding a good way to do that.

[epdenouden]

Some food for thought then @rpkamp and @sebastianbergmann. I will first stomp the ordering bug #3246 and then code a proposal for this.

Unless were are doing double work when CliTestDoxPrinter is also getting an overhaul? Will read up on all discussions before I start coding.

[epdenouden]

With a few reordering issues fixed I will get on this now.

[epdenouden]

To do

• make TestSuiteSorter aware of global test execution order
• find a clean way to pass this test execution order data
  • diff original and post-sorting execution order list
  • pass Δ(normal order, reordered) to TextDox-printer
  • check *TestSuite edge cases to prevent another Fix TestSuite sorting glitch #3408
• update CLITestDoxPrinter
  • make printer aware of the execution sequence while running
  • only cache out of sequence tests: flush asap
• (non-deliverable) experiment: refactor TestDox-printer using hooks interface

[sebastianbergmann]

Do we really need a new CLI option / configuration parameter? We should be able to auto-detect that we need a buffered TestDox printer.

[epdenouden]

We do not need it, no. It would be easy enough pick a behaviour automatically. But I thought while discussing with @rpkamp you agreed you wanted the buffering optional?

So the TestDox printer would offer two modes:

1. buffered: auto-detect if needed based on the diff between 'original order' and 'run order'
2. just output as the hooks are fired

I would happily not implement new configuration items because adding CLI and config options is not all that fun.

[epdenouden]

Tomorrowevening I will start working on the TestSuiteSorter part in any case.

[sebastianbergmann]

By optional I meant "off when not needed" and "on when needed". :)

[epdenouden]

Sorry for the mixup with the regression test yesterday. The one that ended up in #3396 actually belongs here.

I've found a relatively clean way to pass the changes in execution order from the sorter to the printer. Both are instantiated in the TestRunner and getting a running order of tests is not that hard when given a TestSuite to traverse. I'll have a version for review by the end of the weekend.

[epdenouden]

@sebastianbergmann Taking more work than expected to untangle all the interrelated parts of CliTestDoxPrinter, TestResult, etc. Tripped over the printer already buffering additional information like TestSuite headers and non-success stacktraces.

Question: do you mind if I remove the existing buffering done by creating new Test[Dox]Result and include that buffering in the new CliTestDoxPrinter::outputbuffer? Saves a bit of TestResult object juggling and simplifies things. Don't want to spend too many braincells on it, as the old result printers are going to be replaced with a different interface anyway.

Almost works! :)

1) tests/end-to-end/regression/GitHub/3380/issue-3380-test.phpt
Failed asserting that string matches format description.
--- Expected
+++ Actual
@@ @@
 PHPUnit 7.5-g4e22b49ea by Sebastian Bergmann and contributors.
 
-DataproviderExecutionOrder
  ✔ First test that always works
+
  ✔ Add numbers with a dataprovider with data set "1+2=3"
  ✔ Add numbers with a dataprovider with data set "2+1=3"
+DataproviderExecutionOrder
  ✘ Add numbers with a dataprovider with data set "1+1=3"
    │
    │ Failed asserting that 2 is identical to 3.
-   │%w
-   │ %s%etests%e_files%eDataproviderExecutionOrderTest.php:24
-   │%w
+   │ 
+   │ /Users/ewout/proj/phpunit-new-order/tests/_files/DataproviderExecutionOrderTest.php:24
+   │ 
+ ✔ Test in the middle that always works
 
- ✔ Test in the middle that always works
  ✔ Add more numbers with a dataprovider with data set "1+2=3"
  ✔ Add more numbers with a dataprovider with data set "2+1=3"
  ✘ Add more numbers with a dataprovider with data set "1+1=3"
    │
    │ Failed asserting that 2 is identical to 3.
-   │%w
-   │ %s%etests%e_files%eDataproviderExecutionOrderTest.php:37
-   │%w
    │ 
-Time: %s, Memory: %s
+   │ /Users/ewout/proj/phpunit-new-order/tests/_files/DataproviderExecutionOrderTest.php:37
+   │ 
 
+Time: 120 ms, Memory: 6.00MB
+
 Summary of non-successful tests:
-Summary of non-successful tests:

WIP branch is https://github.com/epdenouden/phpunit/tree/issue-3380-buffered-testdox-printer

[sebastianbergmann]

If it works and makes things simpler: go for it :-)

[epdenouden]

Well isn't refactoring fun. It works, putting all functionality back in place and doing some additional cleaning up. Done by end of the week.