Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

W3C Compatability Fixes #372

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 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
20 changes: 16 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ on:
pull_request:
workflow_dispatch:

env:
DRIVER_URL: "http://localhost:4444/wd/hub"
concurrency:
group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }}
cancel-in-progress: true

defaults:
run:
Expand Down Expand Up @@ -43,6 +44,7 @@ jobs:
strategy:
matrix:
php: [ '7.2', '7.3', '7.4', '8.0', '8.1' ]
selenium: ['2.53.1', 'latest']
fail-fast: false

steps:
Expand Down Expand Up @@ -71,16 +73,26 @@ jobs:

- name: Start Selenium
run: |
docker run --net host --name selenium --volume /dev/shm:/dev/shm --shm-size 2g selenium/standalone-firefox:2.53.1 &> ./logs/selenium.log &
docker run --net host --name selenium --volume /dev/shm:/dev/shm --shm-size 2g selenium/standalone-firefox:"${{ matrix.selenium }}" &> ./logs/selenium.log &

- name: Wait for browser & PHP to start
run: |
while ! nc -z localhost 4444 </dev/null; do echo Waiting for remote driver to start...; sleep 1; done
while ! nc -z localhost 8002 </dev/null; do echo Waiting for PHP server to start...; sleep 1; done

- name: Replace instaclick/php-webdriver for new selenium versions
run: |
if [ "${{ matrix.selenium }}" == "latest" ]; then
composer require lullabot/php-webdriver:dev-main
Copy link
Member

Choose a reason for hiding this comment

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

This published fork does not publish the fact that it replaces instaclick/php-webdriver so composer will happily install both in parallel, with no guarantee about which one will win for defining the class. So this looks totally wrong to me.
In the current situation, I'm against merging this PR and releasing as is, because it actually makes the package unreliable for users.

Copy link
Author

Choose a reason for hiding this comment

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

@stof it is marked as replacing it https://github.com/Lullabot/php-webdriver/blob/main/composer.json#L19 and I can see it doing so in the tests I just added:
https://github.com/minkphp/MinkSelenium2Driver/actions/runs/6392945083/job/17351217167?pr=372

Run if [ "latest" == "latest" ]; then
  if [ "latest" == "latest" ]; then
    composer require lullabot/php-webdriver:dev-main
  fi
  shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0}
  env:
    COMPOSER_PROCESS_TIMEOUT: 0
    COMPOSER_NO_INTERACTION: 1
    COMPOSER_NO_AUDIT: 1
./composer.json has been updated
Running composer update lullabot/php-webdriver
Loading composer repositories with package information
Updating dependencies
Lock file operations: 1 install, 0 updates, 1 removal
  - Removing instaclick/php-webdriver (1.4.16)
  - Locking lullabot/php-webdriver (dev-main 3014d58)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 1 removal
  - Downloading lullabot/php-webdriver (dev-main 3014d58)
  - Removing instaclick/php-webdriver (1.4.16)
  - Installing lullabot/php-webdriver (dev-main 3014d58): Extracting archive
Package phpunit/php-token-stream is abandoned, you should avoid using it. No replacement was suggested.
Generating autoload files
26 packages you are using are looking for funding.
Use the `composer fund` command to find out more!

I'll make a proper release of the library before this new test is merged, I just wanted to get tests etc passing here first 😄

Copy link
Member

Choose a reason for hiding this comment

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

ah indeed. By default, packagist shows the info for the latest release, and this is only in the dev version.

fi

- name: Run tests
run: |
vendor/bin/phpunit -v --coverage-clover=coverage.xml
if [ "${{ matrix.selenium }}" == "latest" ]; then
DRIVER_URL="http://localhost:4444" ./vendor/bin/phpunit -v --coverage-clover=coverage.xml
else
DRIVER_URL="http://localhost:4444/wd/hub" ./vendor/bin/phpunit -v --coverage-clover=coverage.xml
fi

- name: Upload coverage
uses: codecov/codecov-action@v2
Expand Down
83 changes: 71 additions & 12 deletions src/Selenium2Driver.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,23 @@ public static function getDefaultCapabilities()
);
}

/**
* Checks if the WebDriver session is W3C compatible.
*
* @return bool
*
* @throws UnknownCommand
justafish marked this conversation as resolved.
Show resolved Hide resolved
*/
public function isW3C() {
try {
// This method doesn't exist in older versions of php-webdriver.
justafish marked this conversation as resolved.
Show resolved Hide resolved
/** @phpstan-ignore-next-line variable.undefined */
return $this->wdSession->isW3C();
} catch (UnknownCommand $e) {
return false;
}
}

/**
* Makes sure that the Syn event library has been injected into the current page,
* and return $this for a fluid interface,
Expand Down Expand Up @@ -317,10 +334,23 @@ private function executeJsOnElement(Element $element, string $script, bool $sync
{
$script = str_replace('{{ELEMENT}}', 'arguments[0]', $script);

$options = array(
'script' => $script,
'args' => array($element),
);
if ($this->isW3C()) {
$options = array(
'script' => $script,
'args' => [
[
'ELEMENT' => $element->getID(),
'element-6066-11e4-a52e-4f735466cecf' => $element->getID(),
]
],
);
}
else {
$options = [
'script' => $script,
'args' => [['ELEMENT' => $element->getID()]],
];
}

if ($sync) {
return $this->getWebDriverSession()->execute($options);
Expand Down Expand Up @@ -549,6 +579,11 @@ public function getAttribute(string $xpath, string $name)
public function getValue(string $xpath)
{
$element = $this->findElement($xpath);
if ($this->isW3C()) {
// This method doesn't exist in older versions of php-webdriver.
/** @phpstan-ignore-next-line variable.undefined */
return $element->property('value');
}
$elementName = strtolower($element->name());
$elementType = strtolower($element->attribute('type') ?: '');

Expand Down Expand Up @@ -658,7 +693,12 @@ public function setValue(string $xpath, $value)
throw new DriverException('Only string values can be used for a file input.');
}

$element->postValue(array('value' => array(strval($value))));
if ($this->isW3C()) {
$element->postValue(array('text' => $value));
}
else {
$element->postValue(array('value' => array($value)));
}

return;
}
Expand All @@ -675,7 +715,12 @@ public function setValue(string $xpath, $value)
$value = str_repeat(Key::BACKSPACE . Key::DELETE, $existingValueLength) . $value;
}

$element->postValue(array('value' => array($value)));
if ($this->isW3C()) {
$element->postValue(array('text' => $value));
}
else {
$element->postValue(array('value' => array($value)));
}
// Remove the focus from the element if the field still has focus in
// order to trigger the change event. By doing this instead of simply
// triggering the change event for the given xpath we ensure that the
Expand Down Expand Up @@ -800,7 +845,12 @@ public function attachFile(string $xpath, string $path)
$remotePath = $path;
}

$element->postValue(array('value' => array($remotePath)));
if ($this->isW3C()) {
$element->postValue(array('text' => $remotePath));
}
else {
$element->postValue(array('value' => array($remotePath)));
}
}

public function isVisible(string $xpath)
Expand Down Expand Up @@ -921,11 +971,20 @@ public function wait(int $timeout, string $condition)

public function resizeWindow(int $width, int $height, ?string $name = null)
{
$window = $this->getWebDriverSession()->window($name ?: 'current');
\assert($window instanceof Window);
$window->postSize(
array('width' => $width, 'height' => $height)
);
if ($this->isW3C()) {
// This method doesn't exist in older versions of php-webdriver.
/** @phpstan-ignore-next-line variable.undefined */
$this->wdSession->window($name ? $name : 'current')->postRect(
justafish marked this conversation as resolved.
Show resolved Hide resolved
array('width' => $width, 'height' => $height)
);
}
else {
$window = $this->getWebDriverSession()->window($name ?: 'current');
\assert($window instanceof Window);
$window->postSize(
array('width' => $width, 'height' => $height)
);
}
}

public function submitForm(string $xpath)
Expand Down
Loading