-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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.
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
Retry upload overwrite if file locked in UI tests #30513
Conversation
Codecov Report
@@ Coverage Diff @@
## master #30513 +/- ##
=========================================
Coverage 61.59% 61.59%
Complexity 18505 18505
=========================================
Files 1090 1090
Lines 61104 61104
=========================================
Hits 37640 37640
Misses 23464 23464 Continue to review full report at Codecov.
|
It can retry the UI upload if it gets a file-locked notification, and the upload works the 2nd time - good.
|
1e98a18
to
b5dc81f
Compare
public function noNotificationShouldBeDisplayed() { | ||
try { | ||
$notificationText = $this->owncloudPage->getNotificationText(); | ||
PHPUnit_Framework_Assert::assertEquals('', $notificationText, "Expecting no notifications but got $notificationText"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pretty long line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will run phpcs
...
try { | ||
$notificationText = $this->owncloudPage->getNotificationText(); | ||
PHPUnit_Framework_Assert::assertEquals('', $notificationText, "Expecting no notifications but got $notificationText"); | ||
} catch (\SensioLabs\Behat\PageObjectExtension\PageObject\Exception\ElementNotFoundException $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we put it in a use
statement?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep
@@ -91,13 +92,15 @@ class FilesContext extends RawMinkContext implements Context { | |||
* @return void | |||
*/ | |||
public function __construct( | |||
OwncloudPage $owncloudPage, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need that? Every page extends OwncloudPage
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not a Page
but a Context
class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
class FilesPage extends FilesPageBasic {
core/tests/ui/features/lib/FilesPage.php
Line 35 in eca03d9
class FilesPage extends FilesPageBasic { |
abstract class FilesPageBasic extends OwnCloudPage {
abstract class FilesPageBasic extends OwnCloudPage { |
so in https://github.com/owncloud/core/pull/30513/files#diff-728c995ac1e138bb2d77c3a588cb018dR481 you should be able to use $notifications = $this->filesPage->getNotifications();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
now I see :) what you mean
eca03d9
to
4fe775e
Compare
changes made - please review again
4fe775e
to
53a6858
Compare
Backport stable10 #30528 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Description
For file upload tests that overwrite an existing file, retry the upload process if it gets some notification (probably about "file locked"). Report that a notification was received, and the text of the notification, so that it is possible to see in the test output that "something happened" but try again anyway up to the "standard" number of retries.
The retry actions go through the same sequence of UI actions that the user would do anyway, when they see a "file locked" message after trying to do an upload.
Related Issue
#30506
Motivation and Context
Make upload-overwrite UI tests reliable.
How Has This Been Tested?
CI passes
Types of changes
Checklist: