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.

Sign up for GitHub

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

Fix tests for Selenium 3.x #60

Closed
wants to merge 5 commits into from
Closed

Fix tests for Selenium 3.x #60

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c8cf063
Test pressed charCode also for keydown event
mvorisek Oct 23, 2022
28280b2
Fix iframe switching test for Selenium 3.x
mvorisek Oct 23, 2022
147493b
Fix input file paths must be reachable from browser
mvorisek Oct 23, 2022
545be83
Fix WindowTest::testWindow test for Selenium 3.x
mvorisek Oct 23, 2022
38c6b3d
address review feedback
mvorisek Nov 18, 2022
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
2 changes: 1 addition & 1 deletion tests/Basic/IFrameTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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


$el = $webAssert->elementExists('css', '#text');
$this->assertSame('iFrame div text', $el->getText());
Expand Down
10 changes: 8 additions & 2 deletions tests/Js/ChangeEventTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

#67 should fix this part.

Copy link
Member

Choose a reason for hiding this comment

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

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

if (getenv('GITHUB_ACTION')) {
$file1 = '/etc/hosts.allow';
$file2 = '/etc/hosts.deny';
} else {
$file1 = realpath(__DIR__ . '/../../web-fixtures/file1.txt');
$file2 = realpath(__DIR__ . '/../../web-fixtures/file2.txt');
}

return array(
'input default' => array('the-input-default', 'from empty', 'from existing'),
Expand Down
6 changes: 3 additions & 3 deletions tests/Js/EventsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 marked this conversation as resolved.
Show resolved Hide resolved

$input2->keyPress('r', $modifier);
$this->assertEquals('key pressed:114 / '.$eventProperties, $event->getText());
$this->assertEquals('key pressed: 114 / ' . $eventProperties, $event->getText());
mvorisek marked this conversation as resolved.
Show resolved Hide resolved

$input3->keyUp(78, $modifier);
$this->assertEquals('key upped:78 / '.$eventProperties, $event->getText());
$this->assertEquals('key upped: 78 / ' . $eventProperties, $event->getText());
mvorisek marked this conversation as resolved.
Show resolved Hide resolved
}

public function provideKeyboardEventsModifiers()
Expand Down
6 changes: 4 additions & 2 deletions tests/Js/WindowTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,21 @@ public function testWindow()
$webAssert = $this->getAssertSession();

$page->clickLink('Popup #1');
$popup1WindowHandle = array_slice($session->getDriver()->getWindowNames(), -1)[0];
$session->switchToWindow(null);

$page->clickLink('Popup #2');
$popup2WindowHandle = array_slice($session->getDriver()->getWindowNames(), -1)[0];
$session->switchToWindow(null);

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

$session->switchToWindow('popup_1');
$session->switchToWindow($popup1WindowHandle);
Copy link
Contributor Author

@mvorisek mvorisek Oct 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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

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

$session->switchToWindow('popup_2');
$session->switchToWindow($popup2WindowHandle);
$el = $webAssert->elementExists('css', '#text');
$this->assertSame('Popup#2 div text', $el->getText());

Expand Down
6 changes: 3 additions & 3 deletions web-fixtures/js_test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just unify/improve keyXxx testing

Copy link
Member

Choose a reason for hiding this comment

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

Please don't add a space after :.

});

$('.elements input.input.second').keypress(function(ev) {
$('.text-event').text('key pressed:' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
$('.text-event').text('key pressed: ' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
mvorisek marked this conversation as resolved.
Show resolved Hide resolved
});

$('.elements input.input.third').keyup(function(ev) {
$('.text-event').text('key upped:' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
$('.text-event').text('key upped: ' + (ev.keyCode || ev.charCode) + ' / ' + ev.altKey * 1 + ' / ' + ev.ctrlKey * 1 + ' / ' + ev.shiftKey * 1 + ' / ' + ev.metaKey * 1);
mvorisek marked this conversation as resolved.
Show resolved Hide resolved
});

$( "#draggable" ).draggable();
Expand Down