From 762a8e0b76e121f108269b916de9d68ff5ffaa9c Mon Sep 17 00:00:00 2001 From: =?utf8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Tue, 2 May 2017 12:49:27 +0200 Subject: [PATCH] Remove "content" locator from acceptance tests MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit The "content" locator uses the "named" Mink selector and the "content" Mink locator to find the element. The "named" Mink first tries to find the elements whose content match exactly the given content but, if none is found, then it tries to find elements that just contain the given content. This behaviour can lead to hard to track issues. Finding the exact match and, if not found, finding the partial match is done in quick succession. In most cases, when looking for an exact match the element is already there, it is returned, and everything works as expected. Or it may not be there, but then it is not there either when finding the partial match, so no element is returned, and everything works as expected (that is, the actor tries to find again the element after some time). However, it can also happen that when looking for an exact match there is no element yet, but it appears after trying to find the exact match but before trying to find the partial match. In that situation the desired element would be returned along with its ancestors. However, as only the first found element is taken into account and the ancestors would appear first the find action would be successful, but the returned element would not be the expected one. This is highly unlikely, yet possible, and can cause sporadic failures in acceptance tests that, apparently, work as expected. Using a "named_exact" Mink selector instead of the "named" Mink selector does not provide the desired behaviour in most cases either. As it finds any element whose content matches exactly the given content, looking for "Hello world" in "

Hello world

" would match the "div", "p" and "a" elements; in that situation the "div" element would be the one returned, when typically the "a" element would be the expected one. As it is error prone and easily replaceable by more robust locators the "content" locator was removed from the predefined ones (although it can still be used if needed through the "customSelector" method in the builder object). Signed-off-by: Daniel Calviño Sánchez --- .../features/bootstrap/FilesAppContext.php | 17 +++++++++++++---- .../bootstrap/FilesSharingAppContext.php | 2 +- .../features/bootstrap/LoginPageContext.php | 2 +- .../features/bootstrap/NotificationContext.php | 3 ++- .../features/bootstrap/SettingsMenuContext.php | 3 ++- tests/acceptance/features/core/Locator.php | 8 -------- 6 files changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/acceptance/features/bootstrap/FilesAppContext.php b/tests/acceptance/features/bootstrap/FilesAppContext.php index bc926fbe52f..e769820c9ee 100644 --- a/tests/acceptance/features/bootstrap/FilesAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesAppContext.php @@ -106,7 +106,11 @@ class FilesAppContext implements Context, ActorAwareInterface { * @return Locator */ public static function shareLinkCheckbox() { - return Locator::forThe()->content("Share link")->descendantOf(self::currentSectionDetailsView())-> + // forThe()->checkbox("Share link") can not be used here; that would + // return the checkbox itself, but the element that the user interacts + // with is the label. + return Locator::forThe()->xpath("//label[normalize-space() = 'Share link']")-> + descendantOf(self::currentSectionDetailsView())-> describedAs("Share link checkbox in the details view in Files app"); } @@ -122,7 +126,11 @@ class FilesAppContext implements Context, ActorAwareInterface { * @return Locator */ public static function passwordProtectCheckbox() { - return Locator::forThe()->content("Password protect")->descendantOf(self::currentSectionDetailsView())-> + // forThe()->checkbox("Password protect") can not be used here; that + // would return the checkbox itself, but the element that the user + // interacts with is the label. + return Locator::forThe()->xpath("//label[normalize-space() = 'Password protect']")-> + descendantOf(self::currentSectionDetailsView())-> describedAs("Password protect checkbox in the details view in Files app"); } @@ -163,7 +171,7 @@ class FilesAppContext implements Context, ActorAwareInterface { * @return Locator */ public static function favoritedStateIconForFile($fileName) { - return Locator::forThe()->content("Favorited")->descendantOf(self::favoriteActionForFile($fileName))-> + return Locator::forThe()->css(".icon-starred")->descendantOf(self::favoriteActionForFile($fileName))-> describedAs("Favorited state icon for file $fileName in Files app"); } @@ -210,7 +218,8 @@ class FilesAppContext implements Context, ActorAwareInterface { * @return Locator */ private static function fileActionsMenuItemFor($itemText) { - return Locator::forThe()->content($itemText)->descendantOf(self::fileActionsMenu())-> + return Locator::forThe()->xpath("//a[normalize-space() = '$itemText']")-> + descendantOf(self::fileActionsMenu())-> describedAs($itemText . " item in file actions menu in Files app"); } diff --git a/tests/acceptance/features/bootstrap/FilesSharingAppContext.php b/tests/acceptance/features/bootstrap/FilesSharingAppContext.php index d9d5eca7359..88c1180c753 100644 --- a/tests/acceptance/features/bootstrap/FilesSharingAppContext.php +++ b/tests/acceptance/features/bootstrap/FilesSharingAppContext.php @@ -47,7 +47,7 @@ class FilesSharingAppContext implements Context, ActorAwareInterface { * @return Locator */ public static function wrongPasswordMessage() { - return Locator::forThe()->content("The password is wrong. Try again.")-> + return Locator::forThe()->xpath("//*[@class = 'warning' and normalize-space() = 'The password is wrong. Try again.']")-> describedAs("Wrong password message in Authenticate page"); } diff --git a/tests/acceptance/features/bootstrap/LoginPageContext.php b/tests/acceptance/features/bootstrap/LoginPageContext.php index 4b0672f652c..61f12f54be6 100644 --- a/tests/acceptance/features/bootstrap/LoginPageContext.php +++ b/tests/acceptance/features/bootstrap/LoginPageContext.php @@ -66,7 +66,7 @@ class LoginPageContext implements Context, ActorAwareInterface { * @return Locator */ public static function wrongPasswordMessage() { - return Locator::forThe()->content("Wrong password. Reset it?")-> + return Locator::forThe()->xpath("//*[@class = 'warning' and normalize-space() = 'Wrong password. Reset it?']")-> describedAs("Wrong password message in Login page"); } diff --git a/tests/acceptance/features/bootstrap/NotificationContext.php b/tests/acceptance/features/bootstrap/NotificationContext.php index f8b784e2465..ac9838562e1 100644 --- a/tests/acceptance/features/bootstrap/NotificationContext.php +++ b/tests/acceptance/features/bootstrap/NotificationContext.php @@ -31,7 +31,8 @@ class NotificationContext implements Context, ActorAwareInterface { * @return Locator */ public static function notificationMessage($message) { - return Locator::forThe()->content($message)->descendantOf(self::notificationContainer())-> + return Locator::forThe()->xpath("//*[@class = 'row' and normalize-space() = '$message']")-> + descendantOf(self::notificationContainer())-> describedAs("$message notification"); } diff --git a/tests/acceptance/features/bootstrap/SettingsMenuContext.php b/tests/acceptance/features/bootstrap/SettingsMenuContext.php index 9ce8df4caef..1ff5d94e98f 100644 --- a/tests/acceptance/features/bootstrap/SettingsMenuContext.php +++ b/tests/acceptance/features/bootstrap/SettingsMenuContext.php @@ -61,7 +61,8 @@ class SettingsMenuContext implements Context, ActorAwareInterface { * @return Locator */ private static function menuItemFor($itemText) { - return Locator::forThe()->content($itemText)->descendantOf(self::settingsMenu())-> + return Locator::forThe()->xpath("//a[normalize-space() = '$itemText']")-> + descendantOf(self::settingsMenu())-> describedAs($itemText . " item in Settings menu"); } diff --git a/tests/acceptance/features/core/Locator.php b/tests/acceptance/features/core/Locator.php index 0ebae9b8fb1..64b2aa00cd9 100644 --- a/tests/acceptance/features/core/Locator.php +++ b/tests/acceptance/features/core/Locator.php @@ -169,14 +169,6 @@ class LocatorBuilder { return $this->customSelector("named", array("link_or_button", $value)); } - /** - * @param string $value - * @return LocatorBuilderSecondStep - */ - public function content($value) { - return $this->customSelector("named", array("content", $value)); - } - /** * @param string $value * @return LocatorBuilderSecondStep -- 2.39.5