Skip to content

Commit 7a8c5ba

Browse files
dunglasdunglas
authored
Fix the test suite some inconsistencies between Chrome and Firefox (#302)
* Fix some inconsistencies between Chrome and Firefox * Fix tests
1 parent 77834cc commit 7a8c5ba

File tree

8 files changed

+113
-22
lines changed

8 files changed

+113
-22
lines changed

‎CHANGELOG.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
CHANGELOG
22
=========
33

4+
0.7.1
5+
-----
6+
7+
* Fix some inconsistencies between Chrome and Firefox
8+
49
0.7.0
510
-----
611

‎src/Client.php‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ final class Client extends AbstractBrowser implements WebDriver, JavaScriptExecu
5252
private $webDriver;
5353
private $browserManager;
5454
private $baseUri;
55+
private $isFirefox = false;
5556

5657
/**
5758
* @param string[]|null $arguments
@@ -89,9 +90,30 @@ public function __destruct()
8990

9091
public function start()
9192
{
92-
if (null === $this->webDriver) {
93-
$this->webDriver = $this->browserManager->start();
93+
if (null !== $this->webDriver) {
94+
return;
95+
}
96+
97+
$this->webDriver = $this->browserManager->start();
98+
if ($this->browserManager instanceof FirefoxManager) {
99+
$this->isFirefox = true;
100+
101+
return;
102+
}
103+
104+
if ($this->browserManager instanceof ChromeManager) {
105+
$this->isFirefox = false;
106+
107+
return;
94108
}
109+
110+
if (method_exists($this->webDriver, 'getCapabilities')) {
111+
$this->isFirefox = 'firefox' === $this->webDriver->getCapabilities()->getBrowserName();
112+
113+
return;
114+
}
115+
116+
$this->isFirefox = false;
95117
}
96118

97119
public function getRequest()
@@ -169,7 +191,30 @@ public function submit(Form $form, array $values = [], array $serverParameters =
169191
}
170192

171193
$button = $form->getButton();
172-
null === $button ? $form->getElement()->submit() : $button->click();
194+
195+
if ($this->isFirefox) {
196+
// For Firefox, we have to wait for the page to reload
197+
// https://github.com/SeleniumHQ/selenium/issues/4570#issuecomment-327473270
198+
$selector = WebDriverBy::cssSelector('html');
199+
$previousId = $this->webDriver->findElement($selector)->getID();
200+
201+
null === $button ? $form->getElement()->submit() : $button->click();
202+
203+
try {
204+
$this->webDriver->wait(5)->until(static function (WebDriver $driver) use ($previousId, $selector) {
205+
try {
206+
return $previousId !== $driver->findElement($selector)->getID();
207+
} catch (NoSuchElementException $e) {
208+
// The html element isn't already available
209+
return false;
210+
}
211+
});
212+
} catch (TimeoutException $e) {
213+
// Probably a form using AJAX, do nothing
214+
}
215+
} else {
216+
null === $button ? $form->getElement()->submit() : $button->click();
217+
}
173218

174219
return $this->crawler = $this->createCrawler();
175220
}

‎src/DomCrawler/Crawler.php‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,11 @@ public function text(string $default = null, bool $normalizeWhitespace = true):
201201
public function html($default = null): string
202202
{
203203
try {
204-
$this->getElementOrThrow();
204+
$element = $this->getElementOrThrow();
205+
206+
if ('html' === $element->getTagName()) {
207+
return $this->webDriver->getPageSource();
208+
}
205209

206210
return $this->attr('outerHTML');
207211
} catch (\InvalidArgumentException $e) {
@@ -227,7 +231,7 @@ public function extract($attributes)
227231
foreach ($this->elements as $element) {
228232
$elements = [];
229233
foreach ($attributes as $attribute) {
230-
$elements[] = '_text' === $attribute ? $element->getText() : $element->getAttribute($attribute);
234+
$elements[] = '_text' === $attribute ? $element->getText() : (string) $element->getAttribute($attribute);
231235
}
232236

233237
$data[] = 1 === $count ? $elements[0] : $elements;
@@ -385,7 +389,10 @@ private function filterWebDriverBy(WebDriverBy $selector): self
385389
{
386390
$subElements = [];
387391
foreach ($this->elements as $element) {
388-
$subElements = \array_merge($subElements, $element->findElements($selector));
392+
$subElements = \array_merge(
393+
$subElements,
394+
$element->findElements($selector)
395+
);
389396
}
390397

391398
return $this->createSubCrawler($subElements);

‎src/PantherTestCaseTrait.php‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919
use Symfony\Component\BrowserKit\HttpBrowser as HttpBrowserClient;
2020
use Symfony\Component\HttpClient\HttpClient;
2121
use Symfony\Component\Panther\Client as PantherClient;
22+
use Symfony\Component\Panther\ProcessManager\ChromeManager;
23+
use Symfony\Component\Panther\ProcessManager\FirefoxManager;
2224
use Symfony\Component\Panther\ProcessManager\WebServerManager;
2325

2426
/**
@@ -160,14 +162,21 @@ public static function isWebServerStarted()
160162
*/
161163
protected static function createPantherClient(array $options = [], array $kernelOptions = []): PantherClient
162164
{
165+
$browser = ($options['browser'] ?? self::$defaultOptions['browser'] ?? self::CHROME);
163166
$callGetClient = \is_callable([self::class, 'getClient']) && (new \ReflectionMethod(self::class, 'getClient'))->isStatic();
164167
if (null !== self::$pantherClient) {
165-
return $callGetClient ? self::getClient(self::$pantherClient) : self::$pantherClient;
168+
$browserManager = self::$pantherClient->getBrowserManager();
169+
if (
170+
(self::CHROME === $browser && $browserManager instanceof ChromeManager) ||
171+
(self::FIREFOX === $browser && $browserManager instanceof FirefoxManager)
172+
) {
173+
return $callGetClient ? self::getClient(self::$pantherClient) : self::$pantherClient;
174+
}
166175
}
167176

168177
self::startWebServer($options);
169178

170-
if (PantherTestCase::CHROME === ($options['browser'] ?? self::$defaultOptions['browser'] ?? PantherTestCase::CHROME)) {
179+
if (self::CHROME === $browser) {
171180
self::$pantherClients[0] = self::$pantherClient = Client::createChromeClient(null, null, [], self::$baseUri);
172181
} else {
173182
self::$pantherClients[0] = self::$pantherClient = Client::createFirefoxClient(null, null, [], self::$baseUri);

‎src/WebDriver/WebDriverCheckbox.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -229,12 +229,12 @@ private function byVisibleText($text, $partial = false, $select = true)
229229
}
230230
}
231231

232-
private function getRelatedElements($value = null)
232+
private function getRelatedElements($value = null): array
233233
{
234234
$valueSelector = $value ? \sprintf(' and @value = %s', XPathEscaper::escapeQuotes($value)) : '';
235235
if (null === $formId = $this->element->getAttribute('form')) {
236236
$form = $this->element->findElement(WebDriverBy::xpath('ancestor::form'));
237-
if ('' === $formId = $form->getAttribute('id')) {
237+
if ('' === $formId = (string) $form->getAttribute('id')) {
238238
return $form->findElements(WebDriverBy::xpath(\sprintf('.//input[@name = %s%s]', XPathEscaper::escapeQuotes($this->name), $valueSelector)));
239239
}
240240
}

‎tests/ClientTest.php‎

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
use Symfony\Component\Panther\Client;
2626
use Symfony\Component\Panther\Cookie\CookieJar;
2727
use Symfony\Component\Panther\DomCrawler\Crawler;
28+
use Symfony\Component\Panther\ProcessManager\ChromeManager;
2829

2930
/**
3031
* @author Kévin Dunglas <[email protected]>
@@ -61,16 +62,10 @@ public function testWaitFor(string $locator)
6162
$this->assertSame('Hello', $crawler->filter('#hello')->text());
6263
}
6364

64-
public function waitForDataProvider(): array
65+
public function waitForDataProvider(): iterable
6566
{
66-
return [
67-
'css selector' => [
68-
'locator' => '#hello',
69-
],
70-
'xpath expression' => [
71-
'locator' => '//*[@id="hello"]',
72-
],
73-
];
67+
yield 'css selector' => ['locator' => '#hello'];
68+
yield 'xpath expression' => ['locator' => '//*[@id="hello"]'];
7469
}
7570

7671
public function testWaitForInvisibleElement(): void
@@ -158,7 +153,7 @@ public function testFollowLink(callable $clientFactory, string $type): void
158153
/**
159154
* @dataProvider clientFactoryProvider
160155
*/
161-
public function testSubmitForm(callable $clientFactory, string $type): void
156+
public function testSubmitForm(callable $clientFactory): void
162157
{
163158
/** @var AbstractBrowser $client */
164159
$client = $clientFactory();
@@ -169,7 +164,7 @@ public function testSubmitForm(callable $clientFactory, string $type): void
169164

170165
$crawler = $client->submit($form);
171166
$this->assertInstanceOf(DomCrawlerCrawler::class, $crawler);
172-
if (Client::class === $type) {
167+
if ($client instanceof Client) {
173168
$this->assertInstanceOf(Crawler::class, $crawler);
174169
}
175170
$this->assertSame(self::$baseUri.'/form-handle.php', $crawler->getUri());
@@ -294,4 +289,18 @@ public function testServerPort(callable $clientFactory): void
294289
$clientFactory();
295290
$this->assertEquals($expectedPort, \mb_substr(self::$baseUri, -4));
296291
}
292+
293+
/**
294+
* @dataProvider clientFactoryProvider
295+
*/
296+
public function testBrowserProvider(callable $clientFactory): void
297+
{
298+
$client = $clientFactory();
299+
if (!$client instanceof Client) {
300+
$this->markTestSkipped();
301+
}
302+
303+
$client->request('GET', self::$baseUri.'/ua.php');
304+
$this->assertStringContainsString($client->getBrowserManager() instanceof ChromeManager ? 'Chrome' : 'Firefox', $client->getPageSource());
305+
}
297306
}

‎tests/DomCrawler/CrawlerTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ public function testExtract(callable $clientFactory): void
240240
$this->assertSame([['', 'Sibling'], ['foo', 'Sibling 2'], ['', 'Sibling 3']], $crawler->filter('main > p')->extract(['class', '_text']));
241241

242242
// Uncomment when https://github.com/symfony/symfony/pull/26433 will be merged
243-
//$this->assertSame([[], [], []], $crawler->filter('main > p')->extract([]));
243+
$this->assertSame([[], [], []], $crawler->filter('main > p')->extract([]));
244244
}
245245

246246
/**

‎tests/fixtures/ua.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Panther project.
5+
*
6+
* (c) Kévin Dunglas <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
declare(strict_types=1);
13+
14+
require __DIR__.'/security-check.php';
15+
16+
echo $_SERVER['HTTP_USER_AGENT'] ?? 'unknown';

0 commit comments

Comments
 (0)