Using composition rather than inheritance for the hookable testers #443

Closed
stof opened this Issue Jan 24, 2014 · 8 comments

2 participants

@stof
Behat member

In https://github.com/Behat/Behat/pull/433/files, you decomposed the hooks out of the testers themselves. However, this has been done using inheritance, which means that as far as the other code is concerned, it is exactly the same. the only different is the file in which methods are defined (or the fact that an extension can break all hooks by changing the service definition to use the parent class, but I don't consider it as a feature addition). I understand why you did it this way: you are decorating a protected method, not the main method of the tester.

But while working on reimplementing https://github.com/Behat/ChainedStepsExtension, I figured out that the best way to reimplement it would be to decorate the testStep method as well, but inside the hookable dispatcher, not outside it (I implemented it outside for now, and it is breaking the reporting by displaying the substeps in reports as well). I would need to be able to run the substeps without dispatching the events for them

When looking at the different testers, here is what happens:

  • Exercise calls runExercice in run and then builds a new result in which most of the info are discarded (it only keeps the result code)
  • SuiteTester builds the environment, calls testSuite and then discards most infos as well
  • FeatureTester calls testFeature and then discards most infos
  • OutlineTester calls testOutline and then discards most infos
  • StepContainerTester isolates the environment, calls testContainer and then discards most infos
  • BackgroundTester::test simply proxies to testBackground
  • StepTester calls testStep and then discards most infos

Because of this discarding of infos before returning, using a decorator is impossible (infos are already lost), which is exactly why the hookable testers were required to use inheritance.
I think a refactoring would be needed here to allow using composition. The handling of the environment would also be a problem (if you decorate the tester to dispatch events, you still want the environment to be build earlier)
I don't have an idea about how it would be done though. What is the reason to discard the infos in the first place ?

As such refactoring would not be BC for extensions, we probably need to try doing it before 3.0 rather than for 3.1 (following semver, we would have to make it 4.0 in such case technically)

@stof
Behat member

Making my code more complex, I managed to avoid reporting substeps in the output. But as they are still executed after the dispatching of the event because of the way the hookable tester is built, the reporting is broken. I would really need a way to decorate the step tester differently

@stof stof referenced this issue in Behat/ChainedStepsExtension Jan 24, 2014
Open

Status of the extension #2

@everzet
Behat member

On it

@everzet
Behat member

@stof I found a way to compose testers. Preparing PR, you'll love it :)

@stof
Behat member

@everzet what is the ETA for this PR ?

@everzet
Behat member

@stof it required bit more work than initially anticipated. I tried 2 different approaches since the first implementation, but finally found one that is even better than the initial one. It's 100% done - I just need to update formatters, because I broke them a bit with this refactoring. I think it's a matter of 2 days. I'll push unfinished work into the temporary branch on wednesday if I didn't finish before that. So that you can check it out.

@everzet
Behat member

@stof it takes a little bit longer because of formatters rewrite, but I'm really-really-really close. Near there. Today-tomorrow hopefully.

@stof
Behat member

@everzet any news ?

@everzet everzet closed this Mar 12, 2014
@everzet
Behat member

Landed in develop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment