[WIP] Testers/formatters refactoring #457
Conversation
Removed notion of events and hooks from the testers, but added instead an extension point (setup routines) so both events and hooks could be added back through their own extensions
EventDispatcher extension now extends testers infrastructure with event- dispatching testers. This is a follow-up to the previous testers refactoring
This commit decouples hooks from both testers and event dispatcher. Now hooks dispatching routine is handled through a simple event subscriber. This is a follow-up to the previous refactoring
Refactored pretty formatter to be an event listening tree. This helps decoupling flow control and presentation logic and keep overall formatter code very clean and precise
|
@stof I still didn't finish (look checkboxes), but it should give you an idea of where is it going. |
|
@docteurklein I think it'll help if you can take a look too :) |
|
@everzet could you temporary change the Travis config so that it can run the suite ? It currently fails entirely because of the missing progress formatter |
| ) { | ||
| $event = new OutlineTested($environment, $feature, $outline, $result); | ||
| $this->eventDispatcher->dispatch(OutlineTested::AFTER, $event); | ||
| $this->baseTester->setUp($environment, $feature, $outline, $skip, $result); |
There was a problem hiding this comment.
shouldn't this call tearDown instead ?
|
@stof overall feeling about refactoring? |
|
@everzet this refactoring looks good to me. I haven't tried to update the ChainedStepsExtension yet, but I'm quite sure it is now possible to implement it. |
| $formatter->getOutputPrinter()->writeln(); | ||
| } | ||
|
|
||
| $this->printArguments($formatter, $step->getArguments()); |
There was a problem hiding this comment.
This method is an exact copy-paste of PrettyStepPrinter::printStep minus the last line. Wouldn't it be possible to avoid the duplication ?
| * | ||
| * @author Konstantin Kudryashov <ever.zet@gmail.com> | ||
| */ | ||
| interface ExceptionResult extends TestResult |
There was a problem hiding this comment.
I think this one is generic enough to be in Testwork rather than Behat. What do you think ?
|
I haven't tried to update the ChainedStepsExtension yet to see whether it can be implemented properly now (from my review, I think it can). However, I like the new architecture. Much more flexible than the previous one. It is unfortunate that you chose to tag Behat as RC for previous releases though. BC breaks between each RC are not really an indicator of being ready for a release... |
|
@stof there were literally two last bits left that I hated about RC - testers and formatters. And this refactoring handles them both. After cleaning codebase up following this refactoring, it's a release. |
|
@everzet I would actually make a RC3 to have a real RC before the stable release to let extension authors update their code for the BC breaks done after RC2. |
|
@stof fully agree. |
|
@stof my plan is:
|
|
@everzet this plan looks good to me |
|
@stof I fixed most of the things in this PR and started working on others in |
|
Merging as is. It will be easier to review small improvements |
[WIP] Testers/formatters refactoring
|
@stof it'll also be much easier with scrutinizer help: https://scrutinizer-ci.com/g/Behat/Behat :) Thanks @schmittjoh |
This is unfinished testers/formatters refactoring. Basically I refactored both testers and formatters to use composition instead of inheritance.
Things to do: