From 8077dbf0977e5b07730a71df83982f6a0e1ee8b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 17 Jan 2021 17:39:35 +0100 Subject: [PATCH 1/9] Add missing waits when finding elements in the acceptance tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As no timeout was specified the elements were tried to be found just once. This caused the steps to fail if the elements did not appear yet in the page when they were tried to be found. Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/bootstrap/AppSettingsContext.php | 6 +++--- .../acceptance/features/bootstrap/AppsManagementContext.php | 2 +- .../acceptance/features/bootstrap/UsersSettingsContext.php | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppSettingsContext.php b/tests/acceptance/features/bootstrap/AppSettingsContext.php index b2b6744a36..d7ee4a2eb0 100644 --- a/tests/acceptance/features/bootstrap/AppSettingsContext.php +++ b/tests/acceptance/features/bootstrap/AppSettingsContext.php @@ -74,7 +74,7 @@ class AppSettingsContext implements Context, ActorAwareInterface { * @Given I open the settings */ public function iOpenTheSettings() { - $this->actor->find(self::appSettingsOpenButton())->click(); + $this->actor->find(self::appSettingsOpenButton(), 10)->click(); } /** @@ -84,11 +84,11 @@ class AppSettingsContext implements Context, ActorAwareInterface { $locator = self::CheckboxInTheSettings($id); // If locator is not visible, fallback to label - if (!$this->actor->find(self::CheckboxInTheSettings($id))->isVisible()) { + if (!$this->actor->find(self::CheckboxInTheSettings($id), 10)->isVisible()) { $locator = self::checkboxLabelInTheSettings($id); } - $this->actor->find($locator)->click(); + $this->actor->find($locator, 10)->click(); } /** diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index 1f3b2dbe28..d86b972c85 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -209,7 +209,7 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @Given /^I see that the "([^"]*)" is disabled$/ */ public function iSeeThatTheIsDisabled($bundle) { - PHPUnit\Framework\Assert::assertEquals('Enable all', $this->actor->find(self::bundleButton($bundle))->getValue()); + PHPUnit\Framework\Assert::assertEquals('Enable all', $this->actor->find(self::bundleButton($bundle), 2)->getValue()); } /** diff --git a/tests/acceptance/features/bootstrap/UsersSettingsContext.php b/tests/acceptance/features/bootstrap/UsersSettingsContext.php index 020da46612..cc542bf2fc 100644 --- a/tests/acceptance/features/bootstrap/UsersSettingsContext.php +++ b/tests/acceptance/features/bootstrap/UsersSettingsContext.php @@ -176,21 +176,21 @@ class UsersSettingsContext implements Context, ActorAwareInterface { * @When I click the New user button */ public function iClickTheNewUserButton() { - $this->actor->find(self::newUserButton())->click(); + $this->actor->find(self::newUserButton(), 10)->click(); } /** * @When I click the :action action in the :user actions menu */ public function iClickTheAction($action, $user) { - $this->actor->find(self::theAction($action, $user))->click(); + $this->actor->find(self::theAction($action, $user), 10)->click(); } /** * @When I open the actions menu for the user :user */ public function iOpenTheActionsMenuOf($user) { - $this->actor->find(self::actionsMenuOf($user))->click(); + $this->actor->find(self::actionsMenuOf($user), 10)->click(); } /** From dd1a554c79c372acc09ca4ac241c6237bfcac038 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 20:17:53 +0100 Subject: [PATCH 2/9] Verify WaitFor::element... results with an assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WaitFor::element... calls only perform the waiting and return whether the condition succeeded or not, but that result needs to be explicitly checked to prevent further steps from being executed if the wait failed. Signed-off-by: Daniel Calviño Sánchez --- .../bootstrap/AppNavigationContext.php | 14 +++++- .../features/bootstrap/AppSettingsContext.php | 7 ++- .../bootstrap/AppsManagementContext.php | 7 ++- .../features/bootstrap/DialogContext.php | 14 +++++- .../bootstrap/UsersSettingsContext.php | 43 ++++++++++++++++--- 5 files changed, 74 insertions(+), 11 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppNavigationContext.php b/tests/acceptance/features/bootstrap/AppNavigationContext.php index df6ed22896..06a1c563b0 100644 --- a/tests/acceptance/features/bootstrap/AppNavigationContext.php +++ b/tests/acceptance/features/bootstrap/AppNavigationContext.php @@ -112,14 +112,24 @@ class AppNavigationContext implements Context, ActorAwareInterface { * @Then I see that the section :section is shown */ public function iSeeThatTheSectionIsShown($section) { - WaitFor::elementToBeEventuallyShown($this->actor, self::appNavigationSectionItemFor($section)); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::appNavigationSectionItemFor($section), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The section $section in the app navigation is not shown yet after $timeout seconds"); + } } /** * @Then I see that the section :section is not shown */ public function iSeeThatTheSectionIsNotShown($section) { - WaitFor::elementToBeEventuallyNotShown($this->actor, self::appNavigationSectionItemFor($section)); + if (!WaitFor::elementToBeEventuallyNotShown( + $this->actor, + self::appNavigationSectionItemFor($section), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The section $section in the app navigation is still shown after $timeout seconds"); + } } /** diff --git a/tests/acceptance/features/bootstrap/AppSettingsContext.php b/tests/acceptance/features/bootstrap/AppSettingsContext.php index d7ee4a2eb0..eda9fe200d 100644 --- a/tests/acceptance/features/bootstrap/AppSettingsContext.php +++ b/tests/acceptance/features/bootstrap/AppSettingsContext.php @@ -95,6 +95,11 @@ class AppSettingsContext implements Context, ActorAwareInterface { * @Then I see that the settings are opened */ public function iSeeThatTheSettingsAreOpened() { - WaitFor::elementToBeEventuallyShown($this->actor, self::appSettingsContent()); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::appSettingsContent(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The app settings are not open yet after $timeout seconds"); + } } } diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index d86b972c85..b692e77ca6 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -164,7 +164,12 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @Then /^I see that there some apps listed from the app store$/ */ public function iSeeThatThereSomeAppsListedFromTheAppStore() { - WaitFor::elementToBeEventuallyShown($this->actor, self::appEntries(), 10); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::appEntries(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The apps from the app store were not shown yet after $timeout seconds"); + } } /** diff --git a/tests/acceptance/features/bootstrap/DialogContext.php b/tests/acceptance/features/bootstrap/DialogContext.php index f9d2d6244e..e2dccf7b11 100644 --- a/tests/acceptance/features/bootstrap/DialogContext.php +++ b/tests/acceptance/features/bootstrap/DialogContext.php @@ -54,13 +54,23 @@ class DialogContext implements Context, ActorAwareInterface { * @Then I see that the confirmation dialog is shown */ public function iSeeThatTheConfirmationDialogIsShown() { - WaitFor::elementToBeEventuallyShown($this->actor, self::theDialog()); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::theDialog(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The confirmation dialog was not shown yet after $timeout seconds"); + } } /** * @Then I see that the confirmation dialog is not shown */ public function iSeeThatTheConfirmationDialogIsNotShown() { - WaitFor::elementToBeEventuallyNotShown($this->actor, self::theDialog()); + if (!WaitFor::elementToBeEventuallyNotShown( + $this->actor, + self::theDialog(), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The confirmation dialog is still shown after $timeout seconds"); + } } } diff --git a/tests/acceptance/features/bootstrap/UsersSettingsContext.php b/tests/acceptance/features/bootstrap/UsersSettingsContext.php index cc542bf2fc..6b0aa78779 100644 --- a/tests/acceptance/features/bootstrap/UsersSettingsContext.php +++ b/tests/acceptance/features/bootstrap/UsersSettingsContext.php @@ -267,14 +267,24 @@ class UsersSettingsContext implements Context, ActorAwareInterface { * @Then I see that the list of users contains the user :user */ public function iSeeThatTheListOfUsersContainsTheUser($user) { - WaitFor::elementToBeEventuallyShown($this->actor, self::rowForUser($user)); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::rowForUser($user), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The user $user in the list of users is not shown yet after $timeout seconds"); + } } /** * @Then I see that the list of users does not contains the user :user */ public function iSeeThatTheListOfUsersDoesNotContainsTheUser($user) { - WaitFor::elementToBeEventuallyNotShown($this->actor, self::rowForUser($user)); + if (!WaitFor::elementToBeEventuallyNotShown( + $this->actor, + self::rowForUser($user), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The user $user in the list of users is still shown after $timeout seconds"); + } } /** @@ -321,8 +331,26 @@ class UsersSettingsContext implements Context, ActorAwareInterface { * @Then I see that the :cell cell for user :user is done loading */ public function iSeeThatTheCellForUserIsDoneLoading($cell, $user) { - WaitFor::elementToBeEventuallyShown($this->actor, self::classCellForUser($cell . ' icon-loading-small', $user)); - WaitFor::elementToBeEventuallyNotShown($this->actor, self::classCellForUser($cell . ' icon-loading-small', $user)); + // It could happen that the cell for the user was done loading and thus + // the loading icon hidden again even before finding the loading icon + // started. Therefore, if the loading icon could not be found it is just + // assumed that it was already hidden again. Nevertheless, this check + // should be done anyway to ensure that the following scenario steps are + // not executed before the cell for the user was done loading. + try { + $this->actor->find(self::classCellForUser($cell . ' icon-loading-small', $user), 1); + } catch (NoSuchElementException $exception) { + echo "The loading icon for user $user was not found after " . (1 * $this->actor->getFindTimeoutMultiplier()) . " seconds, assumming that it was shown and hidden again before the check started and continuing"; + + return; + } + + if (!WaitFor::elementToBeEventuallyNotShown( + $this->actor, + self::classCellForUser($cell . ' icon-loading-small', $user), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The loading icon for user $user is still shown after $timeout seconds"); + } } /** @@ -337,6 +365,11 @@ class UsersSettingsContext implements Context, ActorAwareInterface { * @Then I see that the edit mode is on for user :user */ public function iSeeThatTheEditModeIsOn($user) { - WaitFor::elementToBeEventuallyShown($this->actor, self::editModeOn($user)); + if (!WaitFor::elementToBeEventuallyShown( + $this->actor, + self::editModeOn($user), + $timeout = 10 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The edit mode for user $user in the list of users is not on yet after $timeout seconds"); + } } } From d548d212a25b6c77eebcd3a2bac0631b7a788281 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sat, 6 Mar 2021 01:29:54 +0100 Subject: [PATCH 3/9] Fix delete user acceptance test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding some missing asserts showed that the "delete user" acceptance test was silently failing, as the deletion was not being confirmed in the dialog and thus the user was not being deleted. The dialog button contains a single quote ("user0's"), so the XPath expression had to be adjusted (it seems that it is not possible to escape a single quote in a string enclosed in single quotes in XPath 1.0). Signed-off-by: Daniel Calviño Sánchez --- tests/acceptance/features/bootstrap/DialogContext.php | 2 +- tests/acceptance/features/users.feature | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/acceptance/features/bootstrap/DialogContext.php b/tests/acceptance/features/bootstrap/DialogContext.php index e2dccf7b11..cbd02b3555 100644 --- a/tests/acceptance/features/bootstrap/DialogContext.php +++ b/tests/acceptance/features/bootstrap/DialogContext.php @@ -38,7 +38,7 @@ class DialogContext implements Context, ActorAwareInterface { * @return Locator */ public static function theDialogButton($text) { - return Locator::forThe()->xpath("//button[normalize-space() = '$text']")-> + return Locator::forThe()->xpath("//button[normalize-space() = \"$text\"]")-> descendantOf(self::theDialog())-> describedAs($text . " button of the dialog"); } diff --git a/tests/acceptance/features/users.feature b/tests/acceptance/features/users.feature index 2b6597b728..adb48d658f 100644 --- a/tests/acceptance/features/users.feature +++ b/tests/acceptance/features/users.feature @@ -30,6 +30,7 @@ Feature: users And I open the actions menu for the user user0 And I see that the "Delete user" action in the user0 actions menu is shown When I click the "Delete user" action in the user0 actions menu + And I click the "Delete user0's account" button of the confirmation dialog Then I see that the list of users does not contains the user user0 Scenario: disable a user From 5859458c52c6c9a7e0945af1514b9c17d6acf7a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 21:28:11 +0100 Subject: [PATCH 4/9] Fix identation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../bootstrap/AppsManagementContext.php | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index b692e77ca6..2b53ee3222 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -33,8 +33,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function enableButtonForApp($app) { return Locator::forThe()->button("Enable")-> - descendantOf(self::rowForApp($app))-> - describedAs("Enable button in the app list for $app"); + descendantOf(self::rowForApp($app))-> + describedAs("Enable button in the app list for $app"); } /** @@ -42,8 +42,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function downloadAndEnableButtonForApp($app) { return Locator::forThe()->button("Download and enable")-> - descendantOf(self::rowForApp($app))-> - describedAs("Download & enable button in the app list for $app"); + descendantOf(self::rowForApp($app))-> + describedAs("Download & enable button in the app list for $app"); } /** @@ -51,8 +51,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function disableButtonForApp($app) { return Locator::forThe()->button("Disable")-> - descendantOf(self::rowForApp($app))-> - describedAs("Disable button in the app list for $app"); + descendantOf(self::rowForApp($app))-> + describedAs("Disable button in the app list for $app"); } /** @@ -60,7 +60,7 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function bundleButton($bundle) { return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input")-> - describedAs("Button to enable / disable bundles"); + describedAs("Button to enable / disable bundles"); } /** @@ -76,7 +76,7 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function emptyAppList() { return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@id='apps-list-empty']")-> - describedAs("Empty apps list view"); + describedAs("Empty apps list view"); } /** @@ -84,7 +84,7 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function appEntries() { return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@class='section']")-> - describedAs("Entries in apps list"); + describedAs("Entries in apps list"); } /** @@ -92,8 +92,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function disabledAppEntries() { return Locator::forThe()->button("Disable")-> - descendantOf(self::appEntries())-> - describedAs("Disable button in the app list"); + descendantOf(self::appEntries())-> + describedAs("Disable button in the app list"); } /** @@ -101,8 +101,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function enabledAppEntries() { return Locator::forThe()->button("Enable")-> - descendantOf(self::appEntries())-> - describedAs("Enable button in the app list"); + descendantOf(self::appEntries())-> + describedAs("Enable button in the app list"); } /** @@ -110,7 +110,7 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public static function sidebar() { return Locator::forThe()->xpath("//*[@id=\"app-sidebar\" or contains(@class, 'app-sidebar')]")-> - describedAs("Sidebar in apps management"); + describedAs("Sidebar in apps management"); } From d293751ba7b1d1592f51e54cd584f82012a04a04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 21:33:29 +0100 Subject: [PATCH 5/9] Add locator for apps list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../bootstrap/AppsManagementContext.php | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index 2b53ee3222..449030e81a 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -28,6 +28,14 @@ use Behat\Behat\Context\Context; class AppsManagementContext implements Context, ActorAwareInterface { use ActorAware; + /** + * @return Locator + */ + public static function appsList() { + return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@id='apps-list']")-> + describedAs("Apps list in Apps Management"); + } + /** * @return Locator */ @@ -59,7 +67,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @return Locator */ public static function bundleButton($bundle) { - return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input")-> + return Locator::forThe()->xpath("//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input")-> + descendantOf(self::appsList())-> describedAs("Button to enable / disable bundles"); } @@ -67,7 +76,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @return Locator */ public static function rowForApp($app) { - return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@class='app-name'][normalize-space() = '$app']/..")-> + return Locator::forThe()->xpath("//div[@class='app-name'][normalize-space() = '$app']/..")-> + descendantOf(self::appsList())-> describedAs("Row for app $app in Apps Management"); } @@ -75,7 +85,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @return Locator */ public static function emptyAppList() { - return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@id='apps-list-empty']")-> + return Locator::forThe()->xpath("//div[@id='apps-list-empty']")-> + descendantOf(self::appsList())-> describedAs("Empty apps list view"); } @@ -83,7 +94,8 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @return Locator */ public static function appEntries() { - return Locator::forThe()->xpath("//main[@id='app-content' or contains(@class, 'app-content')]//div[@class='section']")-> + return Locator::forThe()->xpath("//div[@class='section']")-> + descendantOf(self::appsList())-> describedAs("Entries in apps list"); } From 00b38db77fe933e769cfd2bc79c76e6c98231eae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 21:39:37 +0100 Subject: [PATCH 6/9] Find elements through the actor rather than the driver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Actor::find" is a more robust way to look for elements, as it handles some exceptions that may be thrown. Therefore, even if the elements are not actually used and it is only checked whether they exist or not using the actor is the preferred way when possible (and it also makes it consistent with the rest of the acceptance tests). Signed-off-by: Daniel Calviño Sánchez --- .../bootstrap/AppsManagementContext.php | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index 449030e81a..56ec7c6b85 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -45,6 +45,15 @@ class AppsManagementContext implements Context, ActorAwareInterface { describedAs("Enable button in the app list for $app"); } + /** + * @return Locator + */ + public static function enableButtonForAnyApp() { + return Locator::forThe()->button("Enable")-> + descendantOf(self::appsList())-> + describedAs("Enable button in the app list for any app"); + } + /** * @return Locator */ @@ -63,6 +72,15 @@ class AppsManagementContext implements Context, ActorAwareInterface { describedAs("Disable button in the app list for $app"); } + /** + * @return Locator + */ + public static function disableButtonForAnyApp() { + return Locator::forThe()->button("Disable")-> + descendantOf(self::appsList())-> + describedAs("Disable button in the app list for any app"); + } + /** * @return Locator */ @@ -195,16 +213,24 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @Given /^I see that there are only disabled apps$/ */ public function iSeeThatThereAreOnlyDisabledApps() { - $buttons = $this->actor->getSession()->getDriver()->find("//input[@value = 'Disable']"); - PHPUnit\Framework\Assert::assertEmpty($buttons, 'Found disabled apps'); + try { + $this->actor->find(self::disableButtonForAnyApp(), 2); + + PHPUnit_Framework_Assert::fail("Found enabled apps"); + } catch (NoSuchElementException $exception) { + } } /** * @Given /^I see that there are only enabled apps$/ */ public function iSeeThatThereAreOnlyEnabledApps() { - $buttons = $this->actor->getSession()->getDriver()->find("//input[@value = 'Enable']"); - PHPUnit\Framework\Assert::assertEmpty($buttons, 'Found disabled apps'); + try { + $this->actor->find(self::enableButtonForAnyApp(), 2); + + PHPUnit_Framework_Assert::fail("Found disabled apps"); + } catch (NoSuchElementException $exception) { + } } /** From 30c91a730381e45d74f56759547b8f07fe5e1add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 21:57:47 +0100 Subject: [PATCH 7/9] Add explicit locator for "Enable all" bundle button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of looking for the bundle button and then checking its value now the expected value is included in the locator and the button is checked similarly to other elements. No "Disable all" locator was added as it was not currently needed anywhere. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/AppsManagementContext.php | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index 56ec7c6b85..82d4561bd2 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -84,10 +84,10 @@ class AppsManagementContext implements Context, ActorAwareInterface { /** * @return Locator */ - public static function bundleButton($bundle) { - return Locator::forThe()->xpath("//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input")-> + public static function enableAllBundleButton($bundle) { + return Locator::forThe()->xpath("//div[@class='apps-header']/h2[normalize-space() = '$bundle']/input[@value='Enable all']")-> descendantOf(self::appsList())-> - describedAs("Button to enable / disable bundles"); + describedAs("Button to enable bundles"); } /** @@ -245,14 +245,16 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @When /^I enable all apps from the "([^"]*)"$/ */ public function iEnableAllAppsFromThe($bundle) { - $this->actor->find(self::bundleButton($bundle), 2)->click(); + $this->actor->find(self::enableAllBundleButton($bundle), 2)->click(); } /** * @Given /^I see that the "([^"]*)" is disabled$/ */ public function iSeeThatTheIsDisabled($bundle) { - PHPUnit\Framework\Assert::assertEquals('Enable all', $this->actor->find(self::bundleButton($bundle), 2)->getValue()); + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::enableAllBundleButton($bundle), 2)->isVisible() + ); } /** From f92a216dcf80a2e5cfbe2a956dfbc1abd0a1add6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 22:00:22 +0100 Subject: [PATCH 8/9] Assert also element visibility instead of just finding it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Although if the element could not be found an exception would be thrown and the test aborted if an element is in the DOM but hidden it would be found and the test would pass. Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/AppsManagementContext.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppsManagementContext.php b/tests/acceptance/features/bootstrap/AppsManagementContext.php index 82d4561bd2..332c3b020e 100644 --- a/tests/acceptance/features/bootstrap/AppsManagementContext.php +++ b/tests/acceptance/features/bootstrap/AppsManagementContext.php @@ -170,7 +170,9 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public function iSeeThatTheAppHasBeenEnabled($app) { // TODO: Find a way to check if the enable button is removed - $this->actor->find(self::disableButtonForApp($app), 10); + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::disableButtonForApp($app), 10)->isVisible() + ); } /** @@ -178,7 +180,9 @@ class AppsManagementContext implements Context, ActorAwareInterface { */ public function iSeeThatTheAppHasBeenDisabled($app) { // TODO: Find a way to check if the disable button is removed - $this->actor->find(self::enableButtonForApp($app), 10); + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::enableButtonForApp($app), 10)->isVisible() + ); } /** @@ -237,8 +241,12 @@ class AppsManagementContext implements Context, ActorAwareInterface { * @Given /^I see the app bundles$/ */ public function iSeeTheAppBundles() { - $this->actor->find(self::rowForApp('Auditing / Logging'), 2); - $this->actor->find(self::rowForApp('LDAP user and group backend'), 2); + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::rowForApp('Auditing / Logging'), 2)->isVisible() + ); + PHPUnit_Framework_Assert::assertTrue( + $this->actor->find(self::rowForApp('LDAP user and group backend'), 2)->isVisible() + ); } /** From ace7b9dfe1ba95183e3b27368e1407443a42279b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 5 Mar 2021 22:02:46 +0100 Subject: [PATCH 9/9] Find directly the label instead of falling back to it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The input element is always hidden, so the check always ended falling back to the label. Moreover, the label is the element that the user interacts with, so it must be the one used. Signed-off-by: Daniel Calviño Sánchez --- .../acceptance/features/bootstrap/AppSettingsContext.php | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/acceptance/features/bootstrap/AppSettingsContext.php b/tests/acceptance/features/bootstrap/AppSettingsContext.php index eda9fe200d..c9ff57f376 100644 --- a/tests/acceptance/features/bootstrap/AppSettingsContext.php +++ b/tests/acceptance/features/bootstrap/AppSettingsContext.php @@ -81,14 +81,7 @@ class AppSettingsContext implements Context, ActorAwareInterface { * @Given I toggle the :id checkbox in the settings */ public function iToggleTheCheckboxInTheSettingsTo($id) { - $locator = self::CheckboxInTheSettings($id); - - // If locator is not visible, fallback to label - if (!$this->actor->find(self::CheckboxInTheSettings($id), 10)->isVisible()) { - $locator = self::checkboxLabelInTheSettings($id); - } - - $this->actor->find($locator, 10)->click(); + $this->actor->find(self::checkboxLabelInTheSettings($id), 10)->click(); } /**