Skip to content

Wait for the shared link to be set in the acceptance tests - #7605

Merged
rullzer merged 3 commits into
masterfrom
wait-for-the-shared-link-to-be-set-in-the-acceptance-tests
Dec 22, 2017
Merged

Wait for the shared link to be set in the acceptance tests#7605
rullzer merged 3 commits into
masterfrom
wait-for-the-shared-link-to-be-set-in-the-acceptance-tests

Conversation

@danxuliu

Copy link
Copy Markdown
Member

When clicking on Share link in the Sharing tab of the Files app an input field with the link appears. That input field already exists in the DOM, although empty, before clicking on Share link, and when that is done the proper value is set and then the input field is shown.

In the acceptance tests getValue() can return the value of hidden elements too, so as long as an element exists its value is returned without waiting for the field to be visible. Due to this if the test code runs too fast the I write down the shared link step could be executed before the proper value was set, so the shared link got in that case would be an empty value. Then, when the link was tried to be visited no web page would load, and the acceptance test failed with a message like:

And I see that the current page is the shared link I wrote down # FilesSharingAppContext::iSeeThatTheCurrentPageIsTheSharedLinkIWroteDown()
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-''
+'about:blank'

This pull request should prevent those failures from appearing again. Once it is "confirmed" that it works (that is, the failures have not been seen in a while, as unfortunately they are not deterministic :-( ) it should be backported to stable12 too.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
When clicking on "Share link" in the "Sharing" tab of the Files app an
input field with the link appears. That input field already exists in
the DOM, although empty, before clicking on "Share link", and when that
is done the proper value is set and then the input field is shown.
In the acceptance tests "getValue()" can return the value of hidden
elements too, so as long as an element exists its value is returned
without waiting for the field to be visible. Due to this if the test
code runs too fast the "I write down the shared link" step could be
executed before the proper value was set, so the shared link got in that
case would be an empty value, and this would lead to failures when the
following steps were executed.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliudanxuliu added this to the Nextcloud 13 milestone Dec 22, 2017
@codecov

codecovBot commented Dec 22, 2017

Copy link
Copy Markdown

Codecov Report

Merging #7605 into master will increase coverage by <.01%.
The diff coverage is n/a.

@@ Coverage Diff @@## master #7605 +/- ##
============================================
+ Coverage 51.16% 51.17% +<.01% 
Complexity 24886 24886 ============================================
Files 1602 1602 Lines 94750 94750 Branches 1368 1368 ============================================
+ Hits 48480 48486 +6 + Misses 46270 46264 -6
Impacted FilesCoverage ΔComplexity Δ
lib/private/Server.php81.43% <0%> (-0.12%)134% <0%> (ø)
apps/files_trashbin/lib/Trashbin.php72.53% <0%> (+0.24%)136% <0%> (ø)⬇️
core/js/js.js63.55% <0%> (+0.56%)0% <0%> (ø)⬇️
lib/private/Security/CertificateManager.php92.07% <0%> (+0.99%)39% <0%> (ø)⬇️

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes please!

@rullzer
rullzer merged commit 1477b97 into masterDec 22, 2017
@rullzer
rullzer deleted the wait-for-the-shared-link-to-be-set-in-the-acceptance-tests branch December 22, 2017 09:35
@MorrisJobkeMorrisJobke mentioned this pull request Jan 2, 2018
30 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@danxuliu@rullzer@MorrisJobke