Remove use of TestListener

[joesaunderson]

In PHPUnit 8, TestListener and TestListenerDefaultImplenentation are deprecated.

This is preventing PHPUnit_Selenium from supporting PHPUnit 8.*

[joesaunderson]

@thewunder - I will try and pick this up soon to get a 8.* release out too

[luckydonald]

Is this just used in PHPUnit_Extensions_Selenium2TestCase_ScreenshotListener?

[luckydonald]

According to https://github.com/sebastianbergmann/phpunit/issues/3388, "Use the TestHook interfaces instead". More here: https://github.com/sebastianbergmann/phpunit/issues/3390

[joesaunderson]

This does not seem as simple as first thought, the hooks pass through the name of the Test, not the Test class itself, so calling $test->currentScreenshot() is not possible.

Any suggestions?

[luckydonald]

How was the ScreenshotListener used before? Maybe we can do it differently?

[luckydonald]

So, Session::currentScreenshot() calls what is appearntly a raw selenium command?

[luckydonald]

Not being able to access the test seems to be a feature.

Maybe we need to take a step back here.

So what is the use case of that? As far as I can gather

• a) Providing a helper method for writing a png file to disk
• b) Hooking into the system, to make a screenshot of the test on failure/error.

So. a) could be easily refactored out of there to be simply a helper. b) needs a bit more thinking.

[luckydonald]

b)

Maybe we need something like (please excuse the pseudo code)

class ScreenShotOnErrorListener() implements TestHookOrSomething { 
   function construct($test) {
      $test->attachListener($this);
      $this->test = $test;
   } 

   function onFailure(string $test_name_whatever) {
        $this->test->currentScreenshot();
   } 
 }

class SomeTest {
   function onSetup() {
       new ScreenShotOnErrorListener($this);
   }
}

[luckydonald]

b) makes me feel this is a design flaw that we can only access the selenium API from the $this test class.

[luckydonald]

To have asked the question:

Is the ScreenshotListener a feature we need to keep?

[luckydonald]

@joesaunderson How would that be used?

[joesaunderson]

Scratch that, it wouldn’t work at all.

Struggling to work out how the existing listener can be used in the same way.

What are your ideas @thewunder?

[luckydonald]

Note, until we have that fixed, using the fork where I removed the version limitation works as long as this class discussed here isn't used.

[joesaunderson]

Does anyone have further ideas of how we can remove the use of the TestListener

[luckydonald]

When bumping version from 4.x to 7.x that's an incompatible change, so we could just remove it?
So far I have not seen an actual use case.

[SteveDesmond-ca]

+1 for just removing it so we can get a PHPUnit 8 compatible version out the door. Worst case scenario, something similar can be added in an 8.x release.

My use case for screenshots: when a browser test fails (onNotSuccessfulTest()), take a screenshot of the entire page to show the current state at the point of failure, for debugging assistance. This still works fine, as it's happening within a Selenium2TestCase.

[pajon]

Hi guys,

there are already merged PR (#441, #443), we can release 8.0 version. It is only deprecated and all tests are passed, its fine for now.

We must probably fix this until we approach to PHPUnit 9.* (will be released in February 2020)

Is there any solution how to dont remove this feature ?

[thewunder]

I don't see a way to keep this feature, and I'm OK with removing it. @pajon and I discussed wanting to get w3c mode working before we release 8.0

dev-master is compatible with 8.0 if you are feeling adventuresome.

[pajon]

@thewunder

i tried compatibility with w3c, but it will need a huge amount of time and refactor whole library. php-webdriver w3c talk.

If we want ensure backwards compatibility with current JsonWire protocol, we must add way to choose between webdriver protocols.

For me is better release 8.0 version from master and add compatibility in future release (maybe 9.0)