Fix tests for Selenium 3.x #60

mvorisek · 2022-10-23T23:00:08Z

needed for minkphp/MinkSelenium2Driver#354

mvorisek · 2022-10-23T23:01:04Z

web-fixtures/js_test.html

@@ -73,15 +73,15 @@
            });

            $('.elements input.input.first').keydown(function(ev) {
-                $('.text-event').text('key downed:' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
+                $('.text-event').text('key downed: ' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);


just unify/improve keyXxx testing

Please don't add a space after :.

mvorisek · 2022-10-23T23:04:18Z

tests/Js/WindowTest.php

        $session->switchToWindow(null);

        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $session->switchToWindow('popup_1');
+        $session->switchToWindow($popup1WindowHandle);


since Selenium 3.x and newer Firefox names are handles

and only handle must be sent to webdriver, no name is supported (/w or /wo handle) - fixed in instaclick/php-webdriver#119

Should we preserve backwards-compatible behavior by using the window name on Selenium2 and handle on Selenium3?

The ability to switch a window by name on Selenium 3 was fixed in the minkphp/MinkSelenium2Driver#384.

mvorisek · 2022-10-23T23:06:08Z

tests/Js/ChangeEventTest.php

@@ -60,8 +60,14 @@ public function testSetValueChangeEvent($elementId, $valueForEmpty, $valueForFil

    public function setValueChangeEventDataProvider()
    {
-        $file1 = __DIR__ . '/../../web-fixtures/file1.txt';
-        $file2 = __DIR__ . '/../../web-fixtures/file2.txt';
+        // paths must be canonical and reachable from browser


newer browsers, for security reasons, validate the paths if they exist and are canonical, of course from browser POV, thus we must use some paths available within the browser container

This doesn't introduce any BC beaks for the Selenium2?

#67 should fix this part.

The combination of #67 and #88 will fix it.

mvorisek · 2022-10-23T23:07:44Z

tests/Basic/IFrameTest.php

@@ -14,7 +14,7 @@ public function testIFrame()
        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $this->getSession()->switchToIFrame('subframe');
+        $this->getSession()->switchToIFrame(0);


since Selenium 3.x both Chrome and Firefox accepts only iframe index instead of iframe name

passing webdriver element is not supported https://github.com/instaclick/php-webdriver

Should we preserve backwards-compatible behavior by using the iframe name on Selenium2 and index on Selenium3?

The frame switching support by name/id on Selenium 3 was fixed in the minkphp/MinkSelenium2Driver#382.

mvorisek · 2022-10-23T23:16:50Z

PR with test fixes is RTM, I will then submit fixes for the driver itself

tests/Js/EventsTest.php

aik099 · 2022-11-18T10:52:44Z

web-fixtures/js_test.html

@@ -73,15 +73,15 @@
            });

            $('.elements input.input.first').keydown(function(ev) {
-                $('.text-event').text('key downed:' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
+                $('.text-event').text('key downed: ' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);


Please don't add a space after :.

web-fixtures/js_test.html

aik099 · 2022-11-18T14:14:19Z

tests/Basic/IFrameTest.php

@@ -14,7 +14,7 @@ public function testIFrame()
        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $this->getSession()->switchToIFrame('subframe');
+        $this->getSession()->switchToIFrame(0);


Should we preserve backwards-compatible behavior by using the iframe name on Selenium2 and index on Selenium3?

aik099 · 2022-11-18T14:14:47Z

tests/Js/ChangeEventTest.php

@@ -60,8 +60,14 @@ public function testSetValueChangeEvent($elementId, $valueForEmpty, $valueForFil

    public function setValueChangeEventDataProvider()
    {
-        $file1 = __DIR__ . '/../../web-fixtures/file1.txt';
-        $file2 = __DIR__ . '/../../web-fixtures/file2.txt';
+        // paths must be canonical and reachable from browser


This doesn't introduce any BC beaks for the Selenium2?

aik099 · 2022-11-18T14:15:38Z

tests/Js/WindowTest.php

        $session->switchToWindow(null);

        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $session->switchToWindow('popup_1');
+        $session->switchToWindow($popup1WindowHandle);


Should we preserve backwards-compatible behavior by using the window name on Selenium2 and handle on Selenium3?

aik099 · 2024-02-24T16:42:24Z

tests/Basic/IFrameTest.php

@@ -14,7 +14,7 @@ public function testIFrame()
        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $this->getSession()->switchToIFrame('subframe');
+        $this->getSession()->switchToIFrame(0);


The frame switching support by name/id on Selenium 3 was fixed in the minkphp/MinkSelenium2Driver#382.

aik099 · 2024-02-24T16:43:54Z

tests/Js/ChangeEventTest.php

@@ -60,8 +60,14 @@ public function testSetValueChangeEvent($elementId, $valueForEmpty, $valueForFil

    public function setValueChangeEventDataProvider()
    {
-        $file1 = __DIR__ . '/../../web-fixtures/file1.txt';
-        $file2 = __DIR__ . '/../../web-fixtures/file2.txt';
+        // paths must be canonical and reachable from browser


The combination of #67 and #88 will fix it.

aik099 · 2024-02-24T16:45:04Z

tests/Js/WindowTest.php

        $session->switchToWindow(null);

        $el = $webAssert->elementExists('css', '#text');
        $this->assertSame('Main window div text', $el->getText());

-        $session->switchToWindow('popup_1');
+        $session->switchToWindow($popup1WindowHandle);


The ability to switch a window by name on Selenium 3 was fixed in the minkphp/MinkSelenium2Driver#384.

aik099 · 2024-02-24T16:45:41Z

tests/Js/EventsTest.php

@@ -99,13 +99,13 @@ public function testKeyboardEvents($modifier, $eventProperties)
        $event = $webAssert->elementExists('css', '.elements .text-event');

        $input1->keyDown('u', $modifier);
-        $this->assertEquals('key downed:'.$eventProperties, $event->getText());
+        $this->assertEquals('key downed:117 / ' . $eventProperties, $event->getText());


@mvorisek , why this is needed?

key presses asserts the code, but key downed not, the answer is: it is not needed, but wanted to be tested - feel free to cherrypick this from this PR (1st commit)

uuf6429 · 2024-02-24T23:11:37Z

@aik099 @mvorisek considering we are using the same test-suite for selenium 2, 3 and 4 in webdriver-classic-driver, I don't think this PR is relevant any more (or at least, it won't really fix selenium3-specific test problems).

aik099 · 2024-02-25T14:34:01Z

@uuf6429 , as for fixing the part I agree (the driver-specific PRs for almost all issues were created and mentioned in the minkphp/MinkSelenium2Driver#383).

I'm not closing this PR yet, because I'm curious what was the reasoning behind key code adding in the tests.

aik099 · 2024-02-25T18:31:06Z

Closing, because selenium 2 driver code was changed instead (PRs not merged yet) to make tests pass.

mvorisek added 4 commits October 24, 2022 00:55

Test pressed charCode also for keydown event

c8cf063

Fix iframe switching test for Selenium 3.x

28280b2

Fix input file paths must be reachable from browser

147493b

Fix WindowTest::testWindow test for Selenium 3.x

545be83

mvorisek commented Oct 23, 2022

View reviewed changes

aik099 requested changes Nov 18, 2022

View reviewed changes

address review feedback

38c6b3d

mvorisek requested a review from aik099 November 18, 2022 11:05

aik099 requested changes Nov 18, 2022

View reviewed changes

mvorisek mentioned this pull request Mar 1, 2023

Iframe switch - Send element info as selenium 4 require it minkphp/MinkSelenium2Driver#361

Closed

aik099 mentioned this pull request Feb 24, 2024

Fixed file upload test during the build on Selenium 3 #88

Closed

aik099 requested changes Feb 24, 2024

View reviewed changes

aik099 added question response-needed and removed question response-needed labels Feb 25, 2024

aik099 closed this Feb 25, 2024

mvorisek deleted the fix_tests_for_selenium_3 branch February 25, 2024 23:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tests for Selenium 3.x #60

Fix tests for Selenium 3.x #60

mvorisek commented Oct 23, 2022

mvorisek Oct 23, 2022

aik099 Nov 18, 2022

mvorisek Oct 23, 2022 •

edited

Loading

aik099 Nov 18, 2022

aik099 Feb 24, 2024

mvorisek Oct 23, 2022

aik099 Nov 18, 2022

uuf6429 May 27, 2023

aik099 Feb 24, 2024

mvorisek Oct 23, 2022

aik099 Nov 18, 2022

aik099 Feb 24, 2024

mvorisek commented Oct 23, 2022

aik099 Nov 18, 2022

aik099 Nov 18, 2022

aik099 Nov 18, 2022

aik099 Nov 18, 2022

aik099 Feb 24, 2024

aik099 Feb 24, 2024

aik099 Feb 24, 2024

aik099 Feb 24, 2024

mvorisek Feb 25, 2024 •

edited

Loading

uuf6429 commented Feb 24, 2024 •

edited

Loading

aik099 commented Feb 25, 2024

aik099 commented Feb 25, 2024

Fix tests for Selenium 3.x #60

Fix tests for Selenium 3.x #60

Conversation

mvorisek commented Oct 23, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvorisek Oct 23, 2022 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvorisek commented Oct 23, 2022

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

mvorisek Feb 25, 2024 • edited Loading

Choose a reason for hiding this comment

uuf6429 commented Feb 24, 2024 • edited Loading

aik099 commented Feb 25, 2024

aik099 commented Feb 25, 2024

mvorisek Oct 23, 2022 •

edited

Loading

mvorisek Feb 25, 2024 •

edited

Loading

uuf6429 commented Feb 24, 2024 •

edited

Loading