panther
panther copied to clipboard
createPantherClient improvement
Adds ability to pass browser arguments to the client when a Browser has been specified in the options.
According to what we were able to discuss in #428 , there isn't a clear way to configure a browser window size for the browser (assuming we want to test in interactive mode) from createPantherClient.
I commented the following in the issue, looking for answers to this:
It would appear by looking at the source code (if I'm not wrong) that if
I call Client::createChromeClient I won't have started a
WebServer (which I guess it's fine in my case, but not ALL cases) and I
won't boot a kernel. What implications does this have for
the test case?
Regardless of the implications, I think this is probably a good addition: We can reach firefox/chrome options from createPantherClient instead of directly.
Since browser arguments are specific to browser type, I've added a check to nullify this argument IF a browser hasn't been specified by the user. I wonder if this is needed? If I pass a wrong argument to a browser, I suppose the driver will just ignore it and move on.
Let me know if this is a sensible addition or if there is a better way to handle this.
Fixes #428
Isn't this change a BC break?
@dunglas since the new argument is optional and is introduced at the end of the argument list, it should not break anything.
It will. This is a protected method that can be overrode. https://symfony.com/doc/current/contributing/code/bc.html#changing-classes
Will fix soon!
It looks like tests are failing on Ubuntu becuase Chrome Driver crashes, but my local tests in Ubuntu 20.04 with PHP 7.4 are passing.
I've pushed the approach proposed by @nusje2000 . What do you all think?
Any idea why tests are failing left right and center with this error message?
) Symfony\Component\Panther\Tests\AssertionsTest::testDomCrawlerAssertions with data set "Panther" (array('Symfony\Component\Panther\Tes...nsTest', 'createPantherClient'), 'Symfony\Component\Panther\Client')
Facebook\WebDriver\Exception\UnknownErrorException: unknown error: Chrome failed to start: exited abnormally.
(unknown error: DevToolsActivePort file doesn't exist)
(The process started from chrome location /usr/bin/google-chrome is no longer running, so ChromeDriver is assuming that Chrome has crashed.)
(All the tests pass locally in my Ubuntu 20.04 btw)
Alright. One last change. Defaulting browser_arguments to null made all the tests pass! Now also triggering a warning when $options['browser_arguments'] is not an array!
Hi @shadowc! Gentle ping :) Do you still have the time to work on this PR or do you want someone else to try to finish it? Thanks!
I'm so sorry for disappearing! There have been many IRL events that prevented me from doing ANY side work. I WANT to get back to this this weekend if that's ok with you!
No worries, it's perfectly OK to have more important things to do. We aren't paid for this, after all!
Hey, @dunglas ! So now I've corrected the file (also fixed conflicts with latest main) and wrote some tests, although I'm not sure if those are done in a correct way.
I am getting a timeout on PHP 8.0 on Mac which I'm not 100% sure it's unstable or a mistake on the way I'm deleting the client from one test to the next.
Ping @dunglas ! So some unrelated test is failing due to something I cannot really understand (looks like an artifact on the github action? I don't have a Mac unfortunately). Let me know if I can do something this weekend to help with it (have my test broken anything?). (Also my tests might not be what you want/expect, so you can give me some feedback on that as well) :)
Should I Ping again? I assume this PR is still relevant?
This PR is green and ready for review.