owncloud-archive / user_management

GNU Affero General Public License v3.0
2 stars 3 forks source link

Catch waitTillXpathIsVisible() in setQuotaOfUserTo() #163

Closed phil-davis closed 5 years ago

phil-davis commented 5 years ago

1) setQuotaOfUserTo() is responsible for attempting to set the quota of the user on the webUI. When the given quota is expected to be invalid, then it tries to wait for the "invalid" to be actioned on the webUI (so that the page will be "ready" for the next action). It looks for a notification to "pop up".

That logic is fine in general. But if the notification element does not become visible (for whatever reason) then an exception should not be thrown. At this point in the code the purpose of waitTillXpathIsVisible() is only to provide a reasonable wait for the page to be "ready again".

2) when UsersPage loads, we waited for groupListId to be there. That was OK, because that means the page itself has "arrived" properly with the pre-populated groups. But after that there is AJAX that does stuff like:

http://172.17.0.1:8080/index.php/core/ajax/appconfig.php?app=core&key=umgmt_set_password&defaultValue=false&action=getValue
http://172.17.0.1:8080/index.php/apps/user_management/users?offset=0&limit=200&gid=&pattern=

Sometimes we would see and wait for that in waitForOutstandingAjaxCalls() but sometimes we missed it (looking too early...)

So make waitTillPageIsLoaded() wait until it sees at least 1 actual user in the user list - that ensures that we are after the AJAX call to load the first chunk of the user list, and that user(s) are really displayed.

phil-davis commented 5 years ago

A typical job that had the setQuotaOfUserTo() problem is https://drone.owncloud.com/owncloud/user_management/683/56

  Scenario Outline: change quota to an invalid value                                                                # /var/www/owncloud/apps/user_management/tests/acceptance/features/webUIManageQuota/manageUserQuota.feature:38
    When the administrator changes the quota of user "user1" to the invalid string "<wished_quota>" using the webUI # WebUIUsersContext::theAdministratorSetsInvalidQuotaOfUserUsingTheWebUI()
    And the quota of user "user1" should be set to "<wished_quota>" on the webUI                                    # WebUIUsersContext::quotaOfUserShouldBeSetToOnTheWebUI()

    Examples:
      | wished_quota |
      | stupidtext   |
SCENARIO RESULT: (pass)
      | 34,54GB      |
SCENARIO RESULT: (fail)
      | 30/40GB      |
        Exception: Page\OwncloudPage::waitTillXpathIsVisible xpath: //*[@id='notification'] timeout waiting for element to be visible in /var/www/owncloud/tests/acceptance/features/lib/OwncloudPage.php:170
        Stack trace:
        #0 /var/www/owncloud/apps/user_management/tests/acceptance/features/lib/UsersPage.php(548): Page\OwncloudPage->waitTillXpathIsVisible('//*[@id='notifi...')
        #1 /tmp/ProxyManagerGeneratedProxy__PM__PageUsersPageGenerated66e34eaab36d13143e41501a3c46e15f.php(134): Page\UsersPage->setQuotaOfUserTo('user1', '30/40GB', Object(Behat\Mink\Session), false)
        #2 /var/www/owncloud/apps/user_management/tests/acceptance/features/bootstrap/WebUIUsersContext.php(109): ProxyManagerGeneratedProxy\__PM__\Page\UsersPage\Generated66e34eaab36d13143e41501a3c46e15f->setQuotaOfUserTo('user1', '30/40GB', Object(Behat\Mink\Session), false)
        #3 [internal function]: WebUIUsersContext->theAdministratorSetsInvalidQuotaOfUserUsingTheWebUI('user1', '30/40GB')
        #4 /var/www/owncloud/apps/user_management/vendor-bin/behat/vendor/behat/behat/src/Behat/Testwork/Call/Handler/RuntimeCallHandler.php(109): call_user_func_array(Array, Array)
codecov[bot] commented 5 years ago

Codecov Report

Merging #163 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #163   +/-   ##
=========================================
  Coverage     78.71%   78.71%           
  Complexity      214      214           
=========================================
  Files            26       26           
  Lines           916      916           
=========================================
  Hits            721      721           
  Misses          195      195

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3826eb3...1528095. Read the comment docs.

codecov[bot] commented 5 years ago

Codecov Report

Merging #163 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #163   +/-   ##
=========================================
  Coverage     78.71%   78.71%           
  Complexity      214      214           
=========================================
  Files            26       26           
  Lines           916      916           
=========================================
  Hits            721      721           
  Misses          195      195

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 3826eb3...720d9c1. Read the comment docs.