Implement the driver #1

uuf6429 · 2023-06-10T14:59:50Z

Remaining Tasks

~~Update readme~~ Update readme #4
Remove/replace syn.js if possible (Remove syn uuf6429/webdriver-classic-driver#2, Replace syn.js #5)
More tests
- Checking codecov coverage should help
Remove extra public api (done: d385eed)
- Discuss adding it back officially
Investigate random failures (example1, example2)
seems they're always about seleium4+chrome?
Couldn't reproduce them, and last two runs were failure-free - maybe problem got solved somehow?

codecov · 2023-06-10T15:01:42Z

Welcome to Codecov 🎉

Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment.

Thanks for integrating Codecov - We've got you covered ☂️

phpunit.xml.dist

tests/bootstrap.php

src/WebdriverClassicDriver.php

.gitattributes

.github/workflows/ci.yml

src/WebdriverClassicDriver.php

tests/Custom/TimeoutTest.php

src/WebdriverClassicDriver.php

resources/get-syn.php

.github/workflows/ci.yml

phpunit.xml.dist

.github/workflows/ci.yml

aik099 · 2023-07-09T16:56:18Z

phpunit.xml.dist

        <var name="driver_config_factory" value="Mink\WebdriverClassDriver\Tests\WebdriverClassicConfig::getInstance"/>
-        <!--server name="WEB_FIXTURES_HOST" value="http://test.mink.dev" /-->
+
+        <server name="WEB_FIXTURES_HOST" value="http://host.docker.internal:8002"/>
        <!-- MacOS -->
        <!--<server name="WEB_FIXTURES_HOST" value="http://docker.for.mac.localhost:8002"/>-->
-        <!--<server name="WEB_FIXTURES_BROWSER" value="firefox"/>-->
+
        <!-- where driver will connect to -->
        <server name="DRIVER_URL" value="http://localhost:4444/wd/hub"/>
+
        <!-- where DocumentRoot of 'Test Machine' is mounted to on 'Driver Machine' (only if these are 2 different machines) -->
-        <!--server name="DRIVER_MACHINE_BASE_PATH" value="" /-->
-        <!--server name="TEST_MACHINE_BASE_PATH" value="" /-->
+        <server name="DRIVER_MACHINE_BASE_PATH" value="/fixtures/"/>
+
+        <env name="SYMFONY_DEPRECATIONS_HELPER" value="weak"/>


Changing the defaults of this section would result in the developer's inability to run the test suite with a locally started Selenium server.

I replied to this point already, but I guess it got somehow lost: #1 (comment)

That still stands, the recommended test setup is with docker, therefore the defaults are tailored for docker. In case developers want to customize this, they can easily create their own phpunit.xml (that won't be committed with the project).

I'd like a rereview

oleg-andreyev · 2023-07-11T20:40:10Z

tests/WebdriverClassicConfig.php

    protected function supportsCss(): bool
    {
        return true;
    }
+
+    private function isXvfb(): bool


misleading method name. why you need it?

misleading method name

It's not exactly misleading - it's what we are actually intending there by checking the github action env var.

That check was originally implemented in MinkSelenium2Driver:
https://github.com/minkphp/MinkSelenium2Driver/blob/master/tests/Selenium2Config.php#L38

After trial and error, I found what it was trying to do, hence the name I gave it. Maybe we can find a better name though.

Also, it gets its name from here: https://github.com/minkphp/webdriver-classic-driver/pull/1/files/d385eed7f7ec955f3706d765003a5907beb85509#diff-db0e666a5299190f519b12062eb766dccd38dc00a2c6813ec997ed4e6e29b186R45

tests/WebdriverClassicConfig.php

oleg-andreyev · 2023-07-11T20:42:06Z

src/WebdriverClassicDriver.php

+        'chrome' => [
+            'goog:chromeOptions' => [
+                // disable "Chrome is being controlled.." notification bar
+                'excludeSwitches' => ['enable-automation'],


does it affect something?

Not sure what you mean...the comment describes exactly what it does...removing the notification bar means there's more rendering space and less unexpected UI potentially interfering with tests.

The situation for Edge is similar, except that for some weird reason, it refuses to even start when with any settings (but that seems to be something else).

src/WebdriverClassicDriver.php

oleg-andreyev · 2023-07-11T20:47:38Z

src/WebdriverClassicDriver.php

+        $name
+    ): ?string {
+        $escapedName = $this->jsonEncode($name, 'get attribute', 'attribute name');
+        $script = "return arguments[0].getAttribute($escapedName)";


take a look at https://github.com/oleg-andreyev/MinkPhpWebDriver/blob/058c254b04f9586d16fccc82ae2bc0c5e88c402f/src/WebDriver.php#L540-L558

uuf6429 · 2023-09-27T17:25:54Z

src/WebdriverClassicDriver.php

+     * @param string $browserName One of 'edge', 'firefox', 'chrome' or any one of {@see WebDriverBrowserType} constants.
+     */
+    public function __construct(
+        string $browserName = self::DEFAULT_BROWSER,


@stof recently I found that the proper browser name for MSEdge is "MicrosoftEdge".
For some reason, "edge" used to work before, but not anymore.
At the moment, we use the browser name from the docker image suffix, which eventually triggered the problem I just mentioned.

However, this got me thinking, at the moment there aren't any restrictions or suggestions for the browser name from the mink side, which is far from ideal especially since we have extra (default) initialization logic based on the browser name.

On the other hand, I'm not sure it's a good idea to tell the dev to use constants from a 3rd party library (what I sort of the in the phpdoc). I took the liberty to say exactly which browsers are definitely supported in the PHPDoc, which is effectively the keys from BROWSER_NAME_ALIAS_MAP. Another approach could be to define our own constants, or provide them in a new class that extends WebDriverBrowserType.

What do you think?

uuf6429 · 2023-09-27T17:49:08Z

src/WebdriverClassicDriver.php

+
+            case WebDriverBrowserType::MICROSOFT_EDGE:
+                return DesiredCapabilities::microsoftEdge()
+                    ->setCapability(WebDriverCapabilityType::PLATFORM, WebDriverPlatform::ANY);


See: php-webdriver/php-webdriver#1076

stof

Regarding the usage of syn.js, I'd rather keep the initial implementation free of it, marking methods as unsupported and skipping tests if needed.

syn.js is quite a bad fit as it does not actually simulate the actual browser events. See minkphp/driver-testsuite#36 (comment)

Note that we might have to deprecate the existing keypress, keyup and keydown methods as they don't reflect reality: an actual browser has no way to interact in a way that triggers only one of those events.

src/WebdriverClassicDriver.php

+use Facebook\WebDriver\WebDriverDimension;
+use JetBrains\PhpStorm\Language;
+use JsonException;
+use Throwable;


src/WebdriverClassicDriver.php

.github/workflows/ci.yml

stof

Let's finally merge this and keep follow-up works in other PRs

uuf6429 commented Jun 11, 2023

View reviewed changes

phpunit.xml.dist Outdated Show resolved Hide resolved

uuf6429 commented Jun 11, 2023

View reviewed changes

tests/bootstrap.php Show resolved Hide resolved

uuf6429 force-pushed the first-commit branch from f5fc1bb to 1ea3c8b Compare June 11, 2023 06:37

uuf6429 commented Jun 11, 2023

View reviewed changes

src/WebdriverClassicDriver.php Show resolved Hide resolved

stof reviewed Jun 12, 2023

View reviewed changes

stof changed the title ~~First commit~~ Implement the driver Jun 12, 2023

stof previously requested changes Jun 12, 2023

View reviewed changes

src/WebdriverClassicDriver.php Outdated Show resolved Hide resolved

tests/Custom/TimeoutTest.php Outdated Show resolved Hide resolved

src/WebdriverClassicDriver.php Outdated Show resolved Hide resolved

oleg-andreyev reviewed Jun 12, 2023

View reviewed changes

resources/get-syn.php Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

aik099 reviewed Jun 14, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

phpunit.xml.dist Outdated Show resolved Hide resolved

stof reviewed Jun 14, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

uuf6429 added 13 commits June 14, 2023 21:46

First commit

4c0681c

Enable phpunit bridge

ad14001

Reverted license change

6ede658

Improve workflow

c48e7fc

Fix deprecation notice

d4ca830

Remove unused flows/logic

40f600c

Fix exception throwing conformance

0130058

Remove started prop"

b9f6b6f

Guard webDriver prop access

e2c230d

Disable param alignment

c5496ca

Remove continue-on-error

cc7683f

Update test skipping

6196f75

More test skipping fixes

83f0029

uuf6429 force-pushed the first-commit branch from e989b5f to 83f0029 Compare June 14, 2023 20:06

uuf6429 added 2 commits June 14, 2023 22:07

Add jetbrains attribute classes for dev

d30dac8

Improve timeouts test

c62968e

uuf6429 added 3 commits June 17, 2023 21:09

Fix syntax; improve code

be08632

Fix and document regression

b53f9a7

Remove unofficial apis

d385eed

uuf6429 marked this pull request as ready for review June 18, 2023 17:49

uuf6429 requested review from aik099, oleg-andreyev and stof June 18, 2023 17:50

uuf6429 assigned uuf6429 and unassigned uuf6429 Jun 18, 2023

This comment was marked as outdated.

Sign in to view

aik099 reviewed Jul 9, 2023

View reviewed changes

This comment was marked as outdated.

Sign in to view

oleg-andreyev reviewed Jul 11, 2023

View reviewed changes

uuf6429 added 3 commits July 12, 2023 00:37

A few CR fixes

c5edb42

Ensure value for date/time field is a string

6619d75

MSEdge-specific fix

9730f54

uuf6429 commented Sep 27, 2023

View reviewed changes

stof reviewed Sep 27, 2023

View reviewed changes

uuf6429 and others added 3 commits September 28, 2023 12:03

CR fixes

8855250

Various workflow improvements (#1)

9a81918

CR fix

5f52680

This comment was marked as resolved.

Sign in to view

uuf6429 requested a review from stof October 22, 2023 13:02

stof approved these changes Dec 9, 2023

View reviewed changes

stof merged commit 3a116f9 into minkphp:main Dec 9, 2023
29 checks passed

uuf6429 mentioned this pull request Feb 22, 2024

Selenium 4 support minkphp/MinkSelenium2Driver#373

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement the driver #1

Implement the driver #1

uuf6429 commented Jun 10, 2023 •

edited

Loading

codecov bot commented Jun 10, 2023 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

aik099 Jul 9, 2023

uuf6429 Jul 9, 2023 •

edited

Loading

This comment was marked as outdated.

This comment was marked as outdated.

oleg-andreyev Jul 11, 2023

uuf6429 Jul 11, 2023 •

edited

Loading

uuf6429 Jul 11, 2023

oleg-andreyev Jul 11, 2023

uuf6429 Jul 11, 2023

uuf6429 Jul 11, 2023

oleg-andreyev Jul 11, 2023

uuf6429 Sep 27, 2023

uuf6429 Sep 27, 2023

stof left a comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

stof left a comment

Implement the driver #1

Implement the driver #1

Conversation

uuf6429 commented Jun 10, 2023 • edited Loading

codecov bot commented Jun 10, 2023 • edited Loading

Welcome to Codecov 🎉

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

Choose a reason for hiding this comment

uuf6429 Jul 9, 2023 • edited Loading

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

Choose a reason for hiding this comment

uuf6429 Jul 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stof left a comment

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

stof left a comment

Choose a reason for hiding this comment

uuf6429 commented Jun 10, 2023 •

edited

Loading

codecov bot commented Jun 10, 2023 •

edited

Loading

uuf6429 Jul 9, 2023 •

edited

Loading

uuf6429 Jul 11, 2023 •

edited

Loading