Update acceptance tests to Selenium 3 #25991

danxuliu · 2021-03-08T04:17:59Z

This should be rebased on master once #25983 is merged.

The acceptance tests used the last Selenium 2 Docker container available, which provides a rather old Firefox version (Firefox 47). Nevertheless, despite some rendering issues, most things still worked as expected due to the JavaScript files being built with support for older browsers. However, now that support for Internet Explorer 11 and older browsers will be dropped things could start to fail, so a newer browser (and thus a newer Selenium version) should be used in the acceptance tests.

Selenium has been standardized by the W3C, and the protocol to communicate between the Selenium server and the browser has changed due to that. Firefox >= 48 only supports the new W3C protocol, but the Selenium driver for Mink does not support it yet (see issue 293 in https://github.com/minkphp/MinkSelenium2Driver/issues, not linking to prevent polluting the thread with back references).

The old protocol can still be used in recent Chromium/Chrome versions by explicitly forcing it, so for the time being the acceptance tests will need to be run on Chrome instead (although Firefox provides some interesting features like the fake streams that would be needed to test calls in Talk, so they should be moved again to Firefox once possible).

Finally, the default shm size of Docker is 64 MiB. This does not seem enough to run newer Chromium releases and causes the browser to randomly crash during the tests (unknown error: session deleted because of page crash is shown in the logs). Due to this disable-dev-shm-usage needs to be used so Chrome writes shared memory files into /tmp instead of /dev/shm (the default shm size of Docker could have been increased instead using docker run --shm-size..., but that seems to be problematic when the container is run in current Drone releases).

Pending: shm size needs to be increased when run in Drone too. In theory it should be enough to use shm_size, but it seems that parameter no longer works (although I have not actually tested it, I just hoped that for some reason our Drone had a high enough default shm size, but based on the unknown error: session deleted because of page crash errors in the logs it seems it does not :-( )

skjnldsv · 2021-03-30T12:38:25Z

@danxuliu promising results! :)

skjnldsv · 2021-04-12T08:03:15Z

tests/acceptance/features/bootstrap/FilesAppSharingContext.php

 		$this->actor->find(self::passwordProtectCheckbox($shareLinkMenuTriggerElement), 2)->click();

-		$this->actor->find(self::passwordProtectField($shareLinkMenuTriggerElement), 2)->setValue($password . "\r");
+		$this->actor->find(self::passwordProtectField($shareLinkMenuTriggerElement), 2)->setValue($password . Key::ENTER);


PVince81 · 2021-04-12T09:45:03Z

tests/acceptance/run.sh


 	echo "Starting Selenium server"
-	docker run --detach --name=$SELENIUM_CONTAINER --publish 4444:4444 --publish 5900:5900 $DOCKER_OPTIONS selenium/standalone-firefox-debug:2.53.1-beryllium
+	docker run --detach --name=$SELENIUM_CONTAINER --publish 4444:4444 --publish 5900:5900 --shm-size 256m $DOCKER_OPTIONS selenium/standalone-chrome-debug:3.141.59


not sure if you tried this before: https://stackoverflow.com/a/41640458

if it didn't work, there's a note that says that the containers need to be marked as trusted in the repo settings

@PVince81 this file is also used to run selenium locally. So adding a drone-only config would not be compatible :)

ah, missing context. this was an answer to a chat message of @danxuliu where he said if I had an idea how to add it to Drone config. I just picked this line to answer as it was closest :-D

not sure if you tried this before: https://stackoverflow.com/a/41640458

if it didn't work, there's a note that says that the containers need to be marked as trusted in the repo settings

As mentioned in the pull request description I have not tried it yet, but that does not seem to work in latest Drone even if the repository is trusted :-( But I still need to dig into all this ;-)

skjnldsv · 2021-04-13T05:43:15Z

I know, please check the Pending: section in the pull request message :-P That is why it still has the Developing tag ;-)

Well, it was pending for another pr during your vacations. Which I rebased and merged. So I assumed this one was maybe done 🤷

skjnldsv · 2021-04-13T05:44:21Z

Pending: shm size needs to be increased when run in Drone too. In theory it should be enough to use shm_size, but it seems that parameter no longer works

Maybe move the tests to github actions?
Would be much faster

danxuliu · 2021-04-13T08:38:11Z

Well, it was pending for another pr during your vacations. Which I rebased and merged. So I assumed this one was maybe done 🤷

Fair enough ;-)

skjnldsv marked this pull request as draft

🤦 I knew I forgot something xD

Maybe move the tests to github actions?
Would be much faster

I have no idea about GitHub actions :-( But I will take a look, thanks for the suggestion ;-)

Nevertheless, with the current Drone workers the tests are already quite fast. Those that take longer (the sharing ones) are mostly idling in some overcautious waits. I have took a look already at improving them, but failed miserably :-P They are a bit tricky ;-) But I hope that it will be eventually solved :-)

skjnldsv · 2021-04-13T08:41:27Z

I knew I forgot something xD

You didn't ;)

PVince81 · 2021-04-16T15:20:00Z

seems it might be possible to disable shm, see https://developers.google.com/web/tools/puppeteer/troubleshooting#tips

that article points at issues with docker

Sending the "enter" key is not needed in those input fields that auto save while the user is typing or when the focus is lost (which since version 1.4.0 the Selenium driver for Mink is automatically done after setting the value). Signed-off-by: Daniel Calviño Sánchez <[email protected]>

When the value is set in some input fields a carriage return was sent to simulate pressing the enter key and thus confirming the input. However, different browsers use different keys (Firefox uses "\r", but Chrome uses "\n"), so the carriage return was replaced with the WebDriver "ENTER" constant which is common to both browsers. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

The acceptance tests used the last Selenium 2 Docker container available, which provides a rather old Firefox version (Firefox 47). Nevertheless, despite some rendering issues, most things still worked as expected due to the JavaScript files being built with support for older browsers. However, now that support for Internet Explorer 11 and older browsers will be dropped things could start to fail, so a newer browser (and thus a newer Selenium version) should be used in the acceptance tests. Selenium has been standardized by the W3C, and the protocol to communicate between the Selenium server and the browser has changed due to that. Firefox >= 48 only supports the new W3C protocol, but the Selenium driver for Mink does not support it yet. The old protocol can still be used in recent Chromium/Chrome versions by explicitly forcing it, so for the time being the acceptance tests will need to be run on Chrome instead (although Firefox provides some interesting features like the fake streams that would be needed to test calls in Talk, so they should be moved again to Firefox once possible). Finally, the default shm size of Docker is 64 MiB. This does not seem enough to run newer Chrome releases and causes the browser to randomly crash during the tests ("unknown error: session deleted because of page crash" is shown in the logs). Due to this "disable-dev-shm-usage" needs to be used so Chrome writes shared memory files into "/tmp" instead of "/dev/shm" (the default shm size of Docker could have been increased instead using "docker run --shm-size...", but that seems to be problematic when the container is run in current Drone releases). Signed-off-by: Daniel Calviño Sánchez <[email protected]>

skjnldsv · 2021-04-18T07:24:42Z

It worked! 🚀
Only the failing master test is still broken, but we can do this in a followup?

danxuliu · 2021-04-18T20:41:07Z

seems it might be possible to disable shm, see https://developers.google.com/web/tools/puppeteer/troubleshooting#tips

that article points at issues with docker

Great finding, thanks! :-)

It worked! 🚀

Yay! :-D

Only the failing master test is still broken, but we can do this in a followup?

No, luckily the failure from master is fixed now :-) The problem is that, even if Chrome uses the old protocol, in some cases it generates element not interactable errors, which are from the new protocol.

However... I have been running tests literally for hours and the error was never thrown to really investigate it 😢 So I just added a tentative fix for it, but... I do not really know if the tests will always work or randomly fail 🤷

skjnldsv · 2021-04-19T09:59:29Z

continuous-integration/drone/pr — Build is passing 🎉 🚀

skjnldsv · 2021-04-19T09:59:46Z

LGTM

In the WebDriver protocol, when a command fails because it can not interact with the target element, an "element not interactable" error is generated. It can be a transitive issue (for example, due to an animation), so when the error is received the command should be tried again, just like done, for example, with "ElementNotVisible" exceptions. However, the last version of the "instaclick/php-webdriver" library compatible with the Selenium Driver of Mink did not support yet that WebDriver error. And even if Chrome is run using the old protocol an unknown "element not interactable" error can be received anyway in some cases. When an unknown error is received by the "instaclick/php-webdriver" library it is thrown as a generic Exception so, until the library can be updated, the message of generic exceptions is checked and the command is retried if it matched. For the time being "element not interactable" errors are handled like "ElementNotVisible" exceptions; this may need to change once the error is better understood. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu · 2021-04-19T14:40:49Z

continuous-integration/drone/pr — Build is passing 🎉 🚀

The logs also showed some retries on element not interactable messages; that does not guarantee that the fix worked, as that message seems to be shown now also on ElementNotVisible exceptions (:shrug:). But it is likely that it worked, so... I would say merge this and keep an eye on failed tests in the next days ;-)

skjnldsv · 2021-04-21T06:10:06Z

A bit more reviewers @danxuliu ? :)

PVince81

👍

skjnldsv · 2021-04-21T07:31:37Z

Unrelated failure of sqlite

danxuliu · 2021-04-21T08:48:58Z

All acceptance tests currently fail in Drone in pull requests that were branched before this pull request was merged.

It seems that there is a strange mismatch between the Selenium Docker image used and the expected one. I guess it is just some Drone caching issue.

danxuliu · 2021-04-21T17:21:00Z

All acceptance tests currently fail in Drone in pull requests that were branched before this pull request was merged.

It seems that there is a strange mismatch between the Selenium Docker image used and the expected one. I guess it is just some Drone caching issue.

As mentioned it was probably some caching issue, as a pull request rebased after the merge got green CI (such a beautiful colour :-P ).

danxuliu added the 2. developing Work in progress label Mar 8, 2021

danxuliu added this to the Nextcloud 22 milestone Mar 8, 2021

danxuliu mentioned this pull request Mar 8, 2021

Settings: new user row replaced by a modal #23529

Merged

skjnldsv force-pushed the update-acceptance-tests-to-selenium3 branch from 872a5b2 to ecc41bc Compare March 30, 2021 08:32

skjnldsv marked this pull request as ready for review March 30, 2021 08:33

skjnldsv mentioned this pull request Mar 30, 2021

Cleanup old IE/edge properties and implement unsupported browser error page #26358

Merged

skjnldsv approved these changes Apr 12, 2021

View reviewed changes

skjnldsv added enhancement technical debt labels Apr 12, 2021

skjnldsv reviewed Apr 12, 2021

View reviewed changes

PVince81 mentioned this pull request Apr 12, 2021

[master] Consistent sharing acceptance test failure #26314

Closed

PVince81 reviewed Apr 12, 2021

View reviewed changes

skjnldsv force-pushed the update-acceptance-tests-to-selenium3 branch from ecc41bc to dfb1e04 Compare April 12, 2021 10:30

This comment has been minimized.

Sign in to view

skjnldsv marked this pull request as draft April 13, 2021 05:43

danxuliu added 3 commits April 16, 2021 20:00

danxuliu force-pushed the update-acceptance-tests-to-selenium3 branch from dfb1e04 to 9e1246e Compare April 16, 2021 18:00

danxuliu force-pushed the update-acceptance-tests-to-selenium3 branch from 3cdc654 to 4b376a1 Compare April 19, 2021 14:35

danxuliu marked this pull request as ready for review April 19, 2021 14:40

danxuliu added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 19, 2021

skjnldsv requested a review from PVince81 April 21, 2021 06:09

PVince81 approved these changes Apr 21, 2021

View reviewed changes

PVince81 added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 21, 2021

skjnldsv merged commit 31d8820 into master Apr 21, 2021

skjnldsv deleted the update-acceptance-tests-to-selenium3 branch April 21, 2021 07:31

Uh oh!

Update acceptance tests to Selenium 3 #25991

Update acceptance tests to Selenium 3 #25991

Uh oh!

Conversation

danxuliu commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skjnldsv commented Mar 30, 2021

Uh oh!

skjnldsv Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

PVince81 Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

skjnldsv Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

PVince81 Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

danxuliu Apr 12, 2021

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

skjnldsv commented Apr 13, 2021

Uh oh!

skjnldsv commented Apr 13, 2021

Uh oh!

danxuliu commented Apr 13, 2021

Uh oh!

skjnldsv commented Apr 13, 2021

Uh oh!

PVince81 commented Apr 16, 2021

Uh oh!

skjnldsv commented Apr 18, 2021

Uh oh!

danxuliu commented Apr 18, 2021

Uh oh!

skjnldsv commented Apr 19, 2021

Uh oh!

skjnldsv commented Apr 19, 2021

Uh oh!

danxuliu commented Apr 19, 2021

Uh oh!

skjnldsv commented Apr 21, 2021

Uh oh!

PVince81 left a comment

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Apr 21, 2021

Uh oh!

danxuliu commented Apr 21, 2021

Uh oh!

danxuliu commented Apr 21, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

danxuliu commented Mar 8, 2021 •

edited

Loading