Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ jobs:
- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: '7.4' # PHP CS Fixer isn't compatible with PHP 8 yet https://github.com/FriendsOfPHP/PHP-CS-Fixer/issues/4702
php-version: '8.0'
tools: php-cs-fixer, cs2pr

- name: PHP Coding Standards Fixer
Expand Down Expand Up @@ -58,7 +58,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
php-versions: ['7.1', '7.2', '7.3', '7.4', '8.0']
php-versions: ['8.0']
fail-fast: false
name: PHP ${{ matrix.php-versions }} Test on ubuntu-latest
steps:
Expand Down Expand Up @@ -88,6 +88,43 @@ jobs:
- name: Run tests
run: vendor/bin/simple-phpunit

phpunit-dev:
runs-on: ubuntu-latest
strategy:
matrix:
php-versions: ['8.0']
fail-fast: false
name: PHP ${{ matrix.php-versions }} Test dev dependencies on ubuntu-latest
steps:
- name: Checkout
uses: actions/checkout@v2

- name: Setup PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php-versions }}
extensions: zip

- name: Get composer cache directory
id: composercache
run: echo "::set-output name=dir::$(composer config cache-files-dir)"

- name: Cache dependencies
uses: actions/cache@v2
with:
path: ${{ steps.composercache.outputs.dir }}
key: ${{ runner.os }}-composer-${{ hashFiles('**/composer.json') }}
restore-keys: ${{ runner.os }}-composer-

- name: Allow dev dependencies
run: composer config minimum-stability dev
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Normally we have minimum-stability: "dev" in composer.json by default... and that's what we always use. If we did that, then we wouldn't need this second job, I think. For example: https://github.com/symfony/string/blob/d70c35bb20bbca71fc4ab7921e3c6bda1a82a60c/composer.json#L39

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little more info. What we normally do is:

  • Put "minimum-stability": "dev" directly in composer.json
  • For "normal" builds (e.g. released versions like Symfony 5.3), we use --prefer-stable with composer update.
  • For non-released builds (e.g. Symfony 6), we would omit the --prefer-stable.

BUT, I think these are not-important details. The current builds DO test the correct stuff. I vote we merge this, and then we could potentially optimize this build file later to reduce duplication.

Copy link
Contributor Author

@jordisala1991 jordisala1991 Nov 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be nice to have a better testing of different symfony versions, as you said it can be done on another Pr. I can bring some of the workflows we use at Sonata if you dont have a standard way for all sf repositories.


- name: Install dependencies
run: composer install --prefer-dist

- name: Run tests
run: vendor/bin/simple-phpunit

phpunit-lowest:
runs-on: ubuntu-latest
name: PHP 8.0 (lowest) Test on ubuntu-latest
Expand Down
18 changes: 9 additions & 9 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
}
],
"require": {
"php": ">=7.1",
"php": ">=8.0",
"ext-dom": "*",
"ext-libxml": "*",
"php-webdriver/webdriver": "^1.8.2",
"symfony/browser-kit": "^4.4 || ^5.0",
"symfony/browser-kit": "^4.4 || ^5.0 || ^6.0",
"symfony/deprecation-contracts": "^2.4",
"symfony/dom-crawler": "^4.4 || ^5.0",
"symfony/http-client": "^4.4.11 || ^5.2",
"symfony/dom-crawler": "^4.4 || ^5.0 || ^6.0",
"symfony/http-client": "^4.4.11 || ^5.2 || ^6.0",
"symfony/polyfill-php72": "^1.9",
"symfony/process": "^4.4 || ^5.0"
"symfony/process": "^4.4 || ^5.0 || ^6.0"
},
"autoload": {
"psr-4": { "Symfony\\Component\\Panther\\": "src/" }
Expand All @@ -43,9 +43,9 @@
"sort-packages": true
},
"require-dev": {
"symfony/css-selector": "^4.4 || ^5.0",
"symfony/framework-bundle": "^4.4 || ^5.0",
"symfony/mime": "^4.4 || ^5.0",
"symfony/phpunit-bridge": "^5.2"
"symfony/css-selector": "^4.4 || ^5.0 || ^6.0",
"symfony/framework-bundle": "^4.4 || ^5.0 || ^6.0",
"symfony/mime": "^4.4 || ^5.0 || ^6.0",
"symfony/phpunit-bridge": "^5.2 || ^6.0"
}
}
2 changes: 0 additions & 2 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ parameters:
- vendor/bin/.phpunit/phpunit/vendor/autoload.php
inferPrivatePropertyTypeFromConstructor: true
ignoreErrors:
- message: '#.+#'
path: tests/DummyKernel.php
# False positive
- '#Call to an undefined method ReflectionType::getName\(\)\.#'
# To fix in PHP WebDriver
Expand Down
6 changes: 3 additions & 3 deletions src/Client.php
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,12 @@ public function start(): void
$this->isFirefox = false;
}

public function getRequest()
public function getRequest(): object
{
throw new \LogicException('HttpFoundation Request object is not available when using WebDriver.');
}

public function getResponse()
public function getResponse(): object
{
throw new \LogicException('HttpFoundation Response object is not available when using WebDriver.');
}
Expand Down Expand Up @@ -190,7 +190,7 @@ public function setServerParameter($key, $value): void
throw new \InvalidArgumentException('Server parameters cannot be set when using WebDriver.');
}

public function getServerParameter($key, $default = '')
public function getServerParameter($key, $default = ''): mixed
{
throw new \InvalidArgumentException('Server parameters cannot be retrieved when using WebDriver.');
}
Expand Down
32 changes: 16 additions & 16 deletions src/DomCrawler/Crawler.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public function __construct(array $elements = [], ?WebDriver $webDriver = null,
{
$this->uri = $uri;
$this->webDriver = $webDriver;
$this->elements = $elements ?? [];
$this->elements = $elements;
}

public function clear(): void
Expand Down Expand Up @@ -86,7 +86,7 @@ public function addNode(\DOMNode $node): void
throw $this->createNotSupportedException(__METHOD__);
}

public function eq($position): self
public function eq($position): static
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another spot where we can't do this... unless we're planning to drop support for PHP 7. But... maybe we are??? In order to support Symfony 6?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the core team of Symfony should take a decision on this. I can work on both implementations. Keeping support for older php version needs main Symfony repository to be adapted before 6.0 comes out. Removing support for older php versions might need a major version of this package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But indeed this one I missed when reporting here: symfony/symfony#43021

This will be a ton of changes, because all of. this class changed its return types. 😱

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Either we remove the return types from core, or we add some conditional classes/traits declarations in this bundle.
I'm personally ok to revert the addition of these type declarations, we can re-add them later.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, should we revert these in core? If we're going to do that, I guess we should do it as quickly as we can.

(I'm still very unfamiliar with the typing situation - so apologies for not having anything intelligent to add)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I am trying to fix everything here to know what types exactly should be reverted, because I wasn't taking care of static.

My idea is:

  • Prepare this PR to be able to ensure that changes on Symfony will make this PR pass
  • Make a PR on Symfony core to change all types needed here
  • Adapt this PR when the PR on the main repo is merged

For the moment I am having some trouble with PHPStan, since it is not a dependency of this project it got upgraded to 1.0 without notice and now it reports more errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me - thank you and keep us posted :)

Copy link
Contributor Author

@jordisala1991 jordisala1991 Nov 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im trying a little trick to patch symfony to remove its return types so we can ensure that those changes applied to Symfony 6.0 will make build pass with all versions here.

So far I think it will work, but I am having some last troubles with PHPStan. No more problems with PHPStan, it is working.

The idea is to remove the last 2 commits from this PR once I do the Symfony PR to remove the return typehints

wdyt?

{
if (isset($this->elements[$position])) {
return $this->createSubCrawler([$this->elements[$position]]);
Expand All @@ -105,12 +105,12 @@ public function each(\Closure $closure): array
return $data;
}

public function slice($offset = 0, $length = null): self
public function slice($offset = 0, $length = null): static
{
return $this->createSubCrawler(\array_slice($this->elements, $offset, $length));
}

public function reduce(\Closure $closure): self
public function reduce(\Closure $closure): static
{
$elements = [];
foreach ($this->elements as $i => $element) {
Expand All @@ -122,22 +122,22 @@ public function reduce(\Closure $closure): self
return $this->createSubCrawler($elements);
}

public function last(): self
public function last(): static
{
return $this->eq(\count($this->elements) - 1);
}

public function siblings(): self
public function siblings(): static
{
return $this->createSubCrawlerFromXpath('(preceding-sibling::* | following-sibling::*)');
}

public function nextAll(): self
public function nextAll(): static
{
return $this->createSubCrawlerFromXpath('following-sibling::*');
}

public function previousAll(): self
public function previousAll(): static
{
return $this->createSubCrawlerFromXpath('preceding-sibling::*');
}
Expand All @@ -149,15 +149,15 @@ public function parents(): self
return $this->ancestors();
}

public function ancestors(): self
public function ancestors(): static
{
return $this->createSubCrawlerFromXpath('ancestor::*', true);
}

/**
* @see https://github.com/symfony/symfony/issues/26432
*/
public function children(string $selector = null): self
public function children(string $selector = null): static
{
$xpath = 'child::*';
if (null !== $selector) {
Expand Down Expand Up @@ -219,7 +219,7 @@ public function html(string $default = null): string
}
}

public function evaluate($xpath): self
public function evaluate($xpath): static
{
throw $this->createNotSupportedException(__METHOD__);
}
Expand All @@ -241,29 +241,29 @@ public function extract($attributes): array
return $data;
}

public function filterXPath($xpath): self
public function filterXPath($xpath): static
{
return $this->filterWebDriverBy(WebDriverBy::xpath($xpath));
}

public function filter($selector): self
public function filter($selector): static
{
return $this->filterWebDriverBy(WebDriverBy::cssSelector($selector));
}

public function selectLink($value): self
public function selectLink($value): static
{
return $this->selectFromXpath(
sprintf('descendant-or-self::a[contains(concat(\' \', normalize-space(string(.)), \' \'), %1$s) or ./img[contains(concat(\' \', normalize-space(string(@alt)), \' \'), %1$s)]]', self::xpathLiteral(' '.$value.' '))
);
}

public function selectImage($value): self
public function selectImage($value): static
{
return $this->selectFromXpath(sprintf('descendant-or-self::img[contains(normalize-space(string(@alt)), %s)]', self::xpathLiteral($value)));
}

public function selectButton($value): self
public function selectButton($value): static
{
return $this->selectFromXpath(
sprintf(
Expand Down
4 changes: 2 additions & 2 deletions src/DomCrawler/Field/ChoiceFormField.php
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public function untick(): void
$this->setValue(false);
}

public function getValue()
public function getValue(): array|string|null
{
$type = $this->element->getAttribute('type');

Expand Down Expand Up @@ -182,7 +182,7 @@ public function availableOptionValues(): array
/**
* Disables the internal validation of the field.
*/
public function disableValidation(): self
public function disableValidation(): static
{
throw $this->createNotSupportedException(__METHOD__);
}
Expand Down
4 changes: 3 additions & 1 deletion src/DomCrawler/Field/FileFormField.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ final class FileFormField extends BaseFileFormField

/**
* @var array
*
* @phpstan-ignore-next-line
*/
protected $value;

public function getValue()
public function getValue(): array|string|null
{
return $this->value;
}
Expand Down
7 changes: 2 additions & 5 deletions src/DomCrawler/Field/FormFieldTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function __construct(WebDriverElement $element)
$this->initialize();
}

public function getLabel(): void
public function getLabel(): ?\DOMElement
{
throw $this->createNotSupportedException(__METHOD__);
}
Expand All @@ -44,10 +44,7 @@ public function getName(): string
return $this->element->getAttribute('name') ?? '';
}

/**
* @return string|array|null
*/
public function getValue()
public function getValue(): array|string|null
{
return $this->element->getAttribute('value');
}
Expand Down
15 changes: 11 additions & 4 deletions src/DomCrawler/Form.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ public function getFormNode(): \DOMElement
throw $this->createNotSupportedException(__METHOD__);
}

public function setValues(array $values): self
/**
* Disables the internal validation of the field.
*/
public function setValues(array $values): static
{
foreach ($values as $name => $value) {
$this->setValue($name, $value);
Expand Down Expand Up @@ -212,7 +215,12 @@ public function set(FormField $field): void
$this->setValue($field->getName(), $field->getValue());
}

public function get($name)
/**
* @param mixed $name
*
* @return FormField|FormField[]|FormField[][]
*/
public function get($name): FormField|array
{
return $this->getFormField($this->getFormElement($name));
}
Expand All @@ -239,8 +247,7 @@ public function offsetExists($name): bool
*
* @return FormField|FormField[]|FormField[][]
*/
#[\ReturnTypeWillChange]
public function offsetGet($name)
public function offsetGet($name): FormField|array
{
return $this->get($name);
}
Expand Down
2 changes: 1 addition & 1 deletion src/DomCrawler/Link.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public function getElement(): WebDriverElement
return $this->element;
}

public function getNode()
public function getNode(): \DOMElement
{
throw $this->createNotSupportedException(__METHOD__);
}
Expand Down
2 changes: 1 addition & 1 deletion src/ServerTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ private function stopWebServer(): void
private function pause($message): void
{
if (\in_array('--debug', $_SERVER['argv'], true)
&& $_SERVER['PANTHER_NO_HEADLESS'] ?? false
&& ($_SERVER['PANTHER_NO_HEADLESS'] ?? false)
) {
echo "$message\n\nPress enter to continue...";
if (!$this->testing) {
Expand Down
10 changes: 6 additions & 4 deletions src/WebTestAssertionsTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
use Facebook\WebDriver\WebDriverElement;
use Symfony\Bundle\FrameworkBundle\KernelBrowser;
use Symfony\Bundle\FrameworkBundle\Test\WebTestAssertionsTrait as BaseWebTestAssertionsTrait;
use Symfony\Component\BrowserKit\AbstractBrowser;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\Panther\Client as PantherClient;

Expand Down Expand Up @@ -275,9 +274,9 @@ private static function findElement(string $locator): WebDriverElement
* @param array $options An array of options to pass to the createKernel method
* @param array $server An array of server parameters
*
* @return AbstractBrowser A browser instance
* @return KernelBrowser A browser instance
*/
protected static function createClient(array $options = [], array $server = []): AbstractBrowser
protected static function createClient(array $options = [], array $server = []): KernelBrowser
{
$kernel = static::bootKernel($options);

Expand All @@ -293,6 +292,9 @@ protected static function createClient(array $options = [], array $server = []):

$client->setServerParameters($server);

return self::getClient($client);
/** @var KernelBrowser $wrapperClient */
$wrapperClient = self::getClient($client);

return $wrapperClient;
}
}
Loading