MDL-39635 behat: XPath cleanups

- Escaping steps arguments redirected to other steps
- Adding normalized-space() in all contains() assertions
- General xpaths review
- Convering provided xpath text strings to xpath literals
  to avoid problems with arguments containing both single
  quotes and double quotes

admin/tests/behat/behat_admin.php

@@ -72,8 +72,12 @@ public function i_set_the_following_administration_settings_values(TableNode $ta
             // Admin settings does not use the same DOM structure than other moodle forms
             // but we also need to use lib/behat/form_field/* to deal with the different moodle form elements.
             $exception = new ElementNotFoundException($this->getSession(), '"' . $label . '" administration setting ');
+
+            // The argument should be converted to an xpath literal.
+            $label = $this->getSession()->getSelectorsHandler()->xpathLiteral($label);
+
             $fieldxpath = "//*[self::input | self::textarea | self::select][not(./@type = 'submit' or ./@type = 'image' or ./@type = 'hidden')]" .
-                "[@id=//label[contains(normalize-space(string(.)), '" . $label . "')]/@for]";
+                "[@id=//label[contains(normalize-space(.), $label)]/@for]";
             $fieldnode = $this->find('xpath', $fieldxpath, $exception);
             $formfieldtypenode = $this->find('xpath', $fieldxpath . "/ancestor::div[@class='form-setting']" .
                 "/child::div[contains(concat(' ', @class, ' '),  ' form-')]/child::*/parent::div");

admin/tests/behat/display_short_names.feature

@@ -15,7 +15,7 @@ Feature: Display extended course names
     And I should not see "C_shortname Course fullname"
 
   Scenario: Courses list with extended course names
-    Given I click on "Courses" "link" in the "//div[@id='settingsnav']//descendant::li[contains(concat(' ', @class, ' '), ' type_setting ')][not(contains(., 'Site administration'))][contains(., 'Appearance')]" "xpath_element"
+    Given I click on "Courses" "link" in the "//div[@id='settingsnav']/descendant::li[contains(concat(' ', normalize-space(@class), ' '), ' type_setting ')][not(contains(., 'Site administration'))][contains(., 'Appearance')]" "xpath_element"
     And I check "Display extended course names"
     When I press "Save changes"
     And I am on homepage

admin/tool/behat/tests/behat/basic_actions.feature

@@ -37,13 +37,13 @@ Feature: Page contents assertions
     And I follow "Course 1"
     When I click on "Move this to the dock" "button" in the ".block_settings" "css_element"
     Then I should not see "Question bank"
-    And I click on "//div[@id='dock']/descendant::*[contains(., 'Administration')]/h2" "xpath_element"
+    And I click on "//div[@id='dock']/descendant::h2[normalize-space(.)='Administration']" "xpath_element"
 
   @javascript
   Scenario: Locators inside specific DOM nodes using XPath
     Given the following "courses" exists:
       | fullname | shortname | category |
       | Course 1 | C1 | 0 |
     And I log in as "admin"
-    When I click on "Move this to the dock" "button" in the "//*[contains(concat(' ', normalize-space(@class), ' '), ' block_settings ')]" "xpath_element"
+    When I click on "Move this to the dock" "button" in the "//div[contains(concat(' ', normalize-space(@class), ' '), ' block_settings ')]" "xpath_element"
     Then I should not see "Turn editing on"

admin/tool/behat/tests/behat/data_generators.feature

@@ -30,17 +30,17 @@ Feature: Set up contextual data for tests
     Then I should see "Course 1"
     And I should see "Course 2"
     And I should see "Course 3"
-    When I go to the courses management page
+    And I go to the courses management page
     And I follow "Cat 1"
-    Then I should see "Cat 2"
+    And I should see "Cat 2"
     And I should see "Cat 3"
-    When I follow "Cat 3"
-    Then I should see "Course 1"
+    And I follow "Cat 3"
+    And I should see "Course 1"
     And I should see "Course 2"
-    When I select "Cat 2" from "Course categories:"
-    Then I should see "No courses in this category"
-    When I select "Miscellaneous" from "Course categories:"
-    Then I should see "Course 3"
+    And I select "Cat 1 / Cat 2" from "Course categories:"
+    And I should see "No courses in this category"
+    And I select "Miscellaneous" from "Course categories:"
+    And I should see "Course 3"
 
   @javascript
   Scenario: Add a bunch of groups and groupings

auth/tests/behat/behat_auth.php

@@ -50,8 +50,8 @@ public function i_log_in_as($username) {
 
         return array(new Given('I am on homepage'),
             new Given('I follow "' . get_string('login') . '"'),
-            new Given('I fill in "' . get_string('username') . '" with "'.$username.'"'),
-            new Given('I fill in "' . get_string('password') . '" with "'.$username.'"'),
+            new Given('I fill in "' . get_string('username') . '" with "' . $this->escape($username) . '"'),
+            new Given('I fill in "' . get_string('password') . '" with "'. $this->escape($username) . '"'),
             new Given('I press "' . get_string('login') . '"')
         );
     }

backup/util/ui/tests/behat/backup_courses.feature

@@ -31,7 +31,7 @@ Feature: Backup Moodle courses
     And I should not see "Section 3"
     And I press "Continue"
     And I click on "Continue" "button" in the ".bcs-current-course" "css_element"
-    And I click on "//div[contains(concat(' ', @class, ' '), ' fitem ')][contains(., 'Include calendar events')]/descendant::img" "xpath_element"
-    And I click on "setting_root_logs" "checkbox" in the "//div[contains(@class, 'fitem')][contains(., 'Include course logs')]" "xpath_element"
+    And "//div[contains(concat(' ', normalize-space(@class), ' '), ' fitem ')][contains(., 'Include calendar events')]/descendant::img" "xpath_element" should exists
+    And I check "Include course logs"
     And I press "Cancel"
     And I click on "Cancel" "button" in the ".confirmation-dialogue" "css_element"

backup/util/ui/tests/behat/behat_backup.php

@@ -105,15 +105,16 @@ public function i_import_course_into_course($fromcourse, $tocourse, $options = f
         // Click the course link.
         $this->find_link($tocourse)->click();
 
-        // Click the backup link.
+        // Click the import link.
         $this->find_link(get_string('import'))->click();
 
         // Select the course.
         $exception = new ExpectationException('"' . $fromcourse . '" course not found in the list of courses to import from', $this->getSession());
 
-        $fromcourse = str_replace("'", "\'", $fromcourse);
-        $xpath = "//div[contains(concat(' ', @class, ' '), ' ics-results ')]" .
-            "/descendant::tr[contains(., '" . $fromcourse . "')]" .
+        // The argument should be converted to an xpath literal.
+        $fromcourse = $this->getSession()->getSelectorsHandler()->xpathLiteral($fromcourse);
+        $xpath = "//div[contains(concat(' ', normalize-space(@class), ' '), ' ics-results ')]" .
+            "/descendant::tr[contains(., $fromcourse)]" .
             "/descendant::input[@type='radio']";
         $radionode = $this->find('xpath', $xpath, $exception);
         $radionode->check();
@@ -150,17 +151,19 @@ public function i_restore_backup_into_course_using_this_options($backupfilename,
         // Confirm restore.
         $this->select_backup($backupfilename);
 
+        // The argument should be converted to an xpath literal.
+        $existingcourse = $this->getSession()->getSelectorsHandler()->xpathLiteral($existingcourse);
+
         // Selecting the specified course (we can not call behat_forms::select_radio here as is in another behat subcontext).
-        $existingcourse = str_replace("'", "\'", $existingcourse);
-        $radionode = $this->find('xpath', "//div[contains(@class, 'bcs-existing-course')]" .
+        $radionode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), ' bcs-existing-course ')]" .
             "/descendant::div[@class='restore-course-search']" .
-            "/descendant::tr[contains(., '" . $existingcourse . "')]" .
+            "/descendant::tr[contains(., $existingcourse)]" .
             "/descendant::input[@type='radio']");
         $radionode->check();
         $radionode->click();
 
         // Pressing the continue button of the restore into an existing course section.
-        $continuenode = $this->find('xpath', "//div[contains(@class, 'bcs-existing-course')]" .
+        $continuenode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), ' bcs-existing-course ')]" .
             "/descendant::input[@type='submit'][@value='" . get_string('continue') . "']");
         $continuenode->click();
         $this->wait();
@@ -182,14 +185,14 @@ public function i_restore_backup_into_a_new_course_using_this_options($backupfil
         $this->select_backup($backupfilename);
 
         // The first category in the list.
-        $radionode = $this->find('xpath', "//div[contains(@class, 'bcs-new-course')]" .
+        $radionode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), ' bcs-new-course ')]" .
             "/descendant::div[@class='restore-course-search']" .
             "/descendant::input[@type='radio']");
         $radionode->check();
         $radionode->click();
 
         // Pressing the continue button of the restore into an existing course section.
-        $continuenode = $this->find('xpath', "//div[contains(@class, 'bcs-new-course')]" .
+        $continuenode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), ' bcs-new-course ')]" .
             "/descendant::input[@type='submit'][@value='" . get_string('continue') . "']");
         $continuenode->click();
         $this->wait();
@@ -211,13 +214,13 @@ public function i_merge_backup_into_the_current_course($backupfilename, $options
         $this->select_backup($backupfilename);
 
         // Merge without deleting radio option.
-        $radionode = $this->find('xpath', "//div[contains(@class, 'bcs-current-course')]" .
+        $radionode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), 'bcs-current-course')]" .
             "/descendant::input[@type='radio'][@name='target'][@value='1']");
         $radionode->check();
         $radionode->click();
 
         // Pressing the continue button of the restore merging section.
-        $continuenode = $this->find('xpath', "//div[contains(@class, 'bcs-current-course')]" .
+        $continuenode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), 'bcs-current-course')]" .
             "/descendant::input[@type='submit'][@value='" . get_string('continue') . "']");
         $continuenode->click();
         $this->wait();
@@ -239,13 +242,13 @@ public function i_merge_backup_into_current_course_deleting_its_contents($backup
         $this->select_backup($backupfilename);
 
         // Delete contents radio option.
-        $radionode = $this->find('xpath', "//div[contains(@class, 'bcs-current-course')]" .
+        $radionode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), 'bcs-current-course')]" .
             "/descendant::input[@type='radio'][@name='target'][@value='0']");
         $radionode->check();
         $radionode->click();
 
         // Pressing the continue button of the restore merging section.
-        $continuenode = $this->find('xpath', "//div[contains(@class, 'bcs-current-course')]" .
+        $continuenode = $this->find('xpath', "//div[contains(concat(' ', normalize-space(@class), ' '), 'bcs-current-course')]" .
             "/descendant::input[@type='submit'][@value='" . get_string('continue') . "']");
         $continuenode->click();
         $this->wait();
@@ -265,7 +268,11 @@ protected function select_backup($backupfilename) {
 
         // Using xpath as there are other restore links before this one.
         $exception = new ExpectationException('The "' . $backupfilename . '" backup file can not be found in this page', $this->getSession());
-        $xpath = "//tr[contains(., '" . $backupfilename . "')]/descendant::a[contains(., '" . get_string('restore') . "')]";
+
+        // The argument should be converted to an xpath literal.
+        $backupfilename = $this->getSession()->getSelectorsHandler()->xpathLiteral($backupfilename);
+
+        $xpath = "//tr[contains(., $backupfilename)]/descendant::a[contains(., '" . get_string('restore') . "')]";
         $restorelink = $this->find('xpath', $xpath, $exception);
         $restorelink->click();
 
@@ -341,6 +348,10 @@ protected function fill_backup_restore_form($options) {
      */
     protected function wait($timeout = false) {
 
+        if (!$this->running_javascript()) {
+            return;
+        }
+
         if (!$timeout) {
             $timeout = self::TIMEOUT;
         }

blocks/comments/tests/behat/behat_block_comments.php

@@ -91,8 +91,11 @@ public function i_delete_comment_from_comments_block($comment) {
 
         $exception = new ElementNotFoundException($this->getSession(), '"' . $comment . '" comment ');
 
-        $commentxpath = "//div[contains(concat(' ', @class, ' '), ' block_comments ')]" .
-            "/descendant::div[@class='comment-message'][contains(., '" . $comment . "')]";
+        // Using xpath liternal to avoid possible problems with comments containing quotes.
+        $commentliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($comment);
+
+        $commentxpath = "//div[contains(concat(' ', normalize-space(@class), ' '), ' block_comments ')]" .
+            "/descendant::div[@class='comment-message'][contains(., $commentliteral)]";
         $commentnode = $this->find('xpath', $commentxpath, $exception);
 
         // Click on delete icon.
@@ -101,7 +104,7 @@ public function i_delete_comment_from_comments_block($comment) {
         $deleteicon->click();
 
         // Yes confirm.
-        $confirmnode = $this->find('xpath', "//div[@class='comment-delete-confirm']/descendant::a[contains(., 'Yes')]");
+        $confirmnode = $this->find('xpath', "//div[@class='comment-delete-confirm']/descendant::a[contains(., '" . get_string('yes') . "')]");
         $confirmnode->click();
 
         // Wait for the AJAX request.

blocks/tests/behat/behat_blocks.php

@@ -46,7 +46,7 @@ class behat_blocks extends behat_base {
      * @param string $blockname
      */
     public function i_add_the_block($blockname) {
-        $steps = new Given('I select "' . $blockname . '" from "bui_addblock"');
+        $steps = new Given('I select "' . $this->escape($blockname) . '" from "bui_addblock"');
 
         // If we are running without javascript we need to submit the form.
         if (!$this->running_javascript()) {

blocks/tests/behat/configure_block_throughout_site.feature

@@ -32,4 +32,4 @@ Feature: Add and configure blocks throughout the site
     And I press "Save changes"
     And I follow "Course 1"
     # The first block matching the pattern should be top-left block
-    And I should see "Comments" in the "//*[@id='region-pre']/descendant::div[contains(concat(' ', @class, ' '), ' block ')]" "xpath_element"
+    And I should see "Comments" in the "//*[@id='region-pre']/descendant::div[contains(concat(' ', normalize-space(@class), ' '), ' block ')]" "xpath_element"

cohort/tests/behat/behat_cohort.php

@@ -54,7 +54,7 @@ public function i_add_user_to_cohort($username, $cohortidnumber) {
         $userid = $DB->get_field('user', 'id', array('username' => $username));
 
         $steps = array(
-            new Given('I click on "' . get_string('assign', 'cohort') . '" "link" in the "//table[@id=\'cohorts\']//tr[contains(., \'' . $cohortidnumber . '\')]" "xpath_element"'),
+            new Given('I click on "' . get_string('assign', 'cohort') . '" "link" in the "' . $this->escape($cohortidnumber) . '" table row'),
             new Given('I select "' . $userid . '" from "' . get_string('potusers', 'cohort') . '"'),
             new Given('I press "' . get_string('add') . '"'),
             new Given('I press "' . get_string('backtocohorts', 'cohort') . '"')

cohort/tests/behat/upload_cohort_users.feature

@@ -32,11 +32,11 @@ Feature: Upload users to a cohort
     And I press "Upload users"
     And I press "Continue"
     And I follow "Cohorts"
-    And I click on "Assign" "link" in the "//table[@id='cohorts']//tr[contains(., 'Cohort 1')]" "xpath_element"
+    And I click on "Assign" "link" in the "Cohort 1" table row
     Then the "Current users" select box should contain "Tom Jones ([email protected])"
     And the "Current users" select box should contain "Bob Jones ([email protected])"
     And I press "Back to cohorts"
-    And I click on "Assign" "link" in the "//table[@id='cohorts']//tr[contains(., 'Cohort 2')]" "xpath_element"
+    And I click on "Assign" "link" in the "Cohort 2" table row
     And the "Current users" select box should contain "Mary Smith ([email protected])"
     And the "Current users" select box should contain "Alice Smith ([email protected])"
     And I am on homepage

completion/tests/behat/behat_completion.php

@@ -50,12 +50,13 @@ class behat_completion extends behat_base {
     public function user_has_completed_activity($userfullname, $activityname) {
 
         // Will throw an exception if the element can not be hovered.
+        $titleliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($userfullname . ", " . $activityname . ": Completed");
         $xpath = "//table[@id='completion-progress']" .
-            "/descendant::img[contains(@title, '" . $userfullname . ", " . $activityname . ": Completed')]";
+            "/descendant::img[contains(@title, $titleliteral)]";
 
         return array(
             new Given('I go to the current course activity completion report'),
-            new Given('I hover "' . $xpath . '" "xpath_element"')
+            new Given('I hover "' . $this->escape($xpath) . '" "xpath_element"')
         );
     }
 
@@ -68,11 +69,13 @@ public function user_has_completed_activity($userfullname, $activityname) {
      */
     public function user_has_not_completed_activity($userfullname, $activityname) {
 
+        // Will throw an exception if the element can not be hovered.
+        $titleliteral = $this->getSession()->getSelectorsHandler()->xpathLiteral($userfullname . ", " . $activityname . ": Not completed");
         $xpath = "//table[@id='completion-progress']" .
-            "/descendant::img[contains(@title, '" . $userfullname . ", " . $activityname . ": Not completed')]";
+            "/descendant::img[contains(@title, $titleliteral)]";
         return array(
             new Given('I go to the current course activity completion report'),
-            new Given('I hover "' . $xpath . '" "xpath_element"')
+            new Given('I hover "' . $this->escape($xpath) . '" "xpath_element"')
         );
 
         return $steps;
@@ -89,9 +92,9 @@ public function go_to_the_current_course_activity_completion_report() {
 
         // Expand reports node if we can't see the link.
         try {
-            $this->find('xpath', "//*[@id='settingsnav']" .
+            $this->find('xpath', "//div[@id='settingsnav']" .
                 "/descendant::li" .
-                "/descendant::li[not(contains(@class,'collapsed'))]" .
+                "/descendant::li[not(contains(concat(' ', normalize-space(@class), ' '), ' collapsed '))]" .
                 "/descendant::p[contains(., '" . get_string('pluginname', 'report_progress') . "')]");
         } catch (ElementNotFoundException $e) {
             $steps[] = new Given('I expand "' . get_string('reports') . '" node');