From 700682dc40029e5f48cd44b55b10176e81e0ed89 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Thu, 29 Aug 2019 11:09:10 +0800
Subject: [PATCH 1/5] MDL-66550 behat: Normalise selectors better

---
 lib/behat/behat_base.php              | 116 +++++++++++++++-----------
 lib/behat/classes/behat_selectors.php |  44 +++++-----
 2 files changed, 85 insertions(+), 75 deletions(-)

diff --git a/lib/behat/behat_base.php b/lib/behat/behat_base.php
index 5226f2b677a..3e7d9d84432 100644
--- a/lib/behat/behat_base.php
+++ b/lib/behat/behat_base.php
@@ -32,6 +32,7 @@ use Behat\Mink\Exception\DriverException;
 use Behat\Mink\Exception\ExpectationException;
 use Behat\Mink\Exception\ElementNotFoundException;
 use Behat\Mink\Element\NodeElement;
+use Behat\Mink\Element\Element;
 use Behat\Mink\Session;
 
 /**
@@ -115,14 +116,6 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return NodeElement
      */
     protected function find($selector, $locator, $exception = false, $node = false, $timeout = false) {
-
-        // Throw exception, so dev knows it is not supported.
-        if ($selector === 'named') {
-            $exception = 'Using the "named" selector is deprecated as of 3.1. '
-                .' Use the "named_partial" or use the "named_exact" selector instead.';
-            throw new ExpectationException($exception, $this->getSession());
-        }
-
         // Returns the first match.
         $items = $this->find_all($selector, $locator, $exception, $node, $timeout);
         return count($items) ? reset($items) : null;
@@ -137,12 +130,11 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @param string $selector The selector type (css, xpath, named...)
      * @param mixed $locator It depends on the $selector, can be the xpath, a name, a css locator...
      * @param Exception $exception Otherwise we throw expcetion with generic info
-     * @param NodeElement $node Spins around certain DOM node instead of the whole page
+     * @param NodeElement $container Restrict the search to just children of the specified container
      * @param int $timeout Forces a specific time out (in seconds). If 0 is provided the default timeout will be applied.
      * @return array NodeElements list
      */
-    protected function find_all($selector, $locator, $exception = false, $node = false, $timeout = false) {
-
+    protected function find_all($selector, $locator, $exception = false, $container = false, $timeout = false) {
         // Throw exception, so dev knows it is not supported.
         if ($selector === 'named') {
             $exception = 'Using the "named" selector is deprecated as of 3.1. '
@@ -152,7 +144,6 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
 
         // Generic info.
         if (!$exception) {
-
             // With named selectors we can be more specific.
             if (($selector == 'named_exact') || ($selector == 'named_partial')) {
                 $exceptiontype = $locator[0];
@@ -171,12 +162,6 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
             $exception = new ElementNotFoundException($this->getSession(), $exceptiontype, null, $exceptionlocator);
         }
 
-        $params = array('selector' => $selector, 'locator' => $locator);
-        // Pushing $node if required.
-        if ($node) {
-            $params['node'] = $node;
-        }
-
         // How much we will be waiting for the element to appear.
         if (!$timeout) {
             $timeout = self::get_timeout();
@@ -188,24 +173,61 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
             $microsleep = true;
         }
 
+        // Normalise the values in order to perform the search.
+        [
+            'selector' => $selector,
+            'locator' => $locator,
+            'container' => $container,
+        ] = $this->normalise_selector($selector, $locator, $container ?: $this->getSession()->getPage());
+
         // Waits for the node to appear if it exists, otherwise will timeout and throw the provided exception.
         return $this->spin(
-            function($context, $args) {
-
-                // If no DOM node provided look in all the page.
-                if (empty($args['node'])) {
-                    return $context->getSession()->getPage()->findAll($args['selector'], $args['locator']);
-                }
-
-                return $args['node']->findAll($args['selector'], $args['locator']);
-            },
-            $params,
-            $timeout,
-            $exception,
-            $microsleep
+            function() use ($selector, $locator, $container) {
+                return $container->findAll($selector, $locator);
+            }, [], $timeout, $exception, $microsleep
         );
     }
 
+    /**
+     * Normalise the locator and selector.
+     *
+     * @param string $selector The type of thing to search
+     * @param mixed $locator The locator value. Can be an array, but is more likely a string.
+     * @param Element $container An optional container to search within
+     * @return array The selector, locator, and container to search within
+     */
+    public function normalise_selector(string $selector, $locator, Element $container): array {
+        // Normalise the css and xpath selector types.
+        if ('css_element' === $selector) {
+            $selector = 'css';
+        } else if ('xpath_element' === $selector) {
+            $selector = 'xpath';
+        }
+
+        // Convert to named_partial where the selector type is not named_partial, named_exact, xpath, or css.
+        $converttonamed = !$this->getSession()->getSelectorsHandler()->isSelectorRegistered($selector);
+        $converttonamed = $converttonamed && 'xpath' !== $selector;
+        if ($converttonamed) {
+            $allowedpartialselectors = behat_partial_named_selector::get_allowed_selectors();
+            $allowedexactselectors = behat_exact_named_selector::get_allowed_selectors();
+            if (isset($allowedpartialselectors[$selector])) {
+                $locator = behat_selectors::normalise_named_selector($allowedpartialselectors[$selector], $locator);
+                $selector = 'named_partial';
+            } else if (isset($allowedexactselectors[$selector])) {
+                $locator = behat_selectors::normalise_named_selector($allowedexactselectors[$selector], $locator);
+                $selector = 'named_exact';
+            } else {
+                throw new ExpectationException("The '{$selector}' selector type is not registered.", $this);
+            }
+        }
+
+        return [
+            'selector' => $selector,
+            'locator' => $locator,
+            'container' => $container,
+        ];
+    }
+
     /**
      * Finds DOM nodes in the page using named selectors.
      *
@@ -229,27 +251,14 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return NodeElement
      */
     public function __call($name, $arguments) {
-
-        if (substr($name, 0, 5) !== 'find_') {
-            throw new coding_exception('The "' . $name . '" method does not exist');
+        if (substr($name, 0, 5) === 'find_') {
+            return call_user_func_array([$this, 'find'], array_merge(
+                [substr($name, 5)],
+                $arguments
+            ));
         }
 
-        // Only the named selector identifier.
-        $cleanname = substr($name, 5);
-
-        // All named selectors shares the interface.
-        if (count($arguments) !== 1) {
-            throw new coding_exception('The "' . $cleanname . '" named selector needs the locator as it\'s single argument');
-        }
-
-        // Redirecting execution to the find method with the specified selector.
-        // It will detect if it's pointing to an unexisting named selector.
-        return $this->find('named_partial',
-            array(
-                $cleanname,
-                behat_context_helper::escape($arguments[0])
-            )
-        );
+        throw new coding_exception("The '{$name}' method does not exist");
     }
 
     /**
@@ -433,7 +442,12 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
             throw new ExpectationException('The "' . $selectortype . '" selector type does not exist', $this->getSession());
         }
 
-        return behat_selectors::get_behat_selector($selectortype, $element, $this->getSession());
+        [
+            'selector' => $selector,
+            'locator' => $locator,
+        ] = $this->normalise_selector($selectortype, $element, $this->getSession()->getPage());
+
+        return [$selector, $locator];
     }
 
     /**
diff --git a/lib/behat/classes/behat_selectors.php b/lib/behat/classes/behat_selectors.php
index 04e255ac5fc..5f03e6c59f9 100644
--- a/lib/behat/classes/behat_selectors.php
+++ b/lib/behat/classes/behat_selectors.php
@@ -47,32 +47,14 @@ class behat_selectors {
      * @return array Contains the selector and the locator expected by Mink.
      */
     public static function get_behat_selector($selectortype, $element, Behat\Mink\Session $session) {
+        // Note: This function is not deprecated, but not the recommended way of doing things.
+        [
+            'selector' => $selector,
+            'locator' => $locator,
+        ] = $session->normalise_selector($selectortype, $element, $session->getPage());
 
         // CSS and XPath selectors locator is one single argument.
-        if ($selectortype == 'css_element' || $selectortype == 'xpath_element') {
-            $selector = str_replace('_element', '', $selectortype);
-            $locator = $element;
-        } else {
-            // Named selectors uses arrays as locators including the type of named selector.
-            $allowedselectors = self::get_allowed_selectors();
-            if (!isset($allowedselectors[$selectortype])) {
-                throw new ExpectationException('The "' . $selectortype . '" selector not registered.', $session);
-            }
-            $locator = array($allowedselectors[$selectortype], behat_context_helper::escape($element));
-
-            // Get the selector which should be used.
-            $allowedpartialselectors = behat_partial_named_selector::get_allowed_selectors();
-            $allowedexactselectors = behat_exact_named_selector::get_allowed_selectors();
-            if (isset($allowedpartialselectors[$selectortype])) {
-                $selector = 'named_partial';
-            } else if (isset($allowedexactselectors[$selectortype])) {
-                $selector = 'named_exact';
-            } else {
-                throw new ExpectationException('The "' . $selectortype . '" selector not registered.', $session);
-            }
-        }
-
-        return array($selector, $locator);
+        return [$selector, $locator];
     }
 
     /**
@@ -98,4 +80,18 @@ class behat_selectors {
             behat_exact_named_selector::get_allowed_text_selectors()
         );
     }
+
+    /**
+     * Normalise the selector and locator for a named partial.
+     *
+     * @param string $selector The selector name
+     * @param string $locator The value to normalise
+     * @return array
+     */
+    public static function normalise_named_selector(string $selector, string $locator): array {
+        return [
+            $selector,
+            behat_context_helper::escape($locator),
+        ];
+    }
 }

From b983ae4ab847bee6aee1b7ad9c514c388443a760 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Tue, 10 Sep 2019 21:29:00 +0800
Subject: [PATCH 2/5] MDL-66550 behat: Add ability to transform a specific
 selector

---
 lib/behat/behat_base.php | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/behat/behat_base.php b/lib/behat/behat_base.php
index 3e7d9d84432..c42d38ef5d7 100644
--- a/lib/behat/behat_base.php
+++ b/lib/behat/behat_base.php
@@ -197,6 +197,18 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return array The selector, locator, and container to search within
      */
     public function normalise_selector(string $selector, $locator, Element $container): array {
+        // Check for specific transformations for this selector type.
+        $transformfunction = "transform_find_for_{$selector}";
+        if (method_exists('behat_selectors', $transformfunction)) {
+            // A selector-specific transformation exists.
+            // Perform initial transformation of the selector within the current container.
+            [
+                'selector' => $selector,
+                'locator' => $locator,
+                'container' => $container,
+            ] = behat_selectors::{$transformfunction}($this, $locator, $container);
+        }
+
         // Normalise the css and xpath selector types.
         if ('css_element' === $selector) {
             $selector = 'css';

From d10ed4d240410068fae6315de353f5b9e26450f6 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Thu, 29 Aug 2019 11:39:21 +0800
Subject: [PATCH 3/5] MDL-66550 behat: Standardise finding in core
 functionality

---
 .../behat/behat_block_site_main_menu.php      |   8 +-
 .../behat/behat_block_social_activities.php   |   8 +-
 course/tests/behat/behat_course.php           |   7 +-
 lib/behat/behat_base.php                      |  71 ++++++------
 lib/tests/behat/behat_general.php             | 104 ++++++++----------
 5 files changed, 81 insertions(+), 117 deletions(-)

diff --git a/blocks/site_main_menu/tests/behat/behat_block_site_main_menu.php b/blocks/site_main_menu/tests/behat/behat_block_site_main_menu.php
index f5231162059..b80713c60c6 100644
--- a/blocks/site_main_menu/tests/behat/behat_block_site_main_menu.php
+++ b/blocks/site_main_menu/tests/behat/behat_block_site_main_menu.php
@@ -114,12 +114,8 @@ class behat_block_site_main_menu extends behat_base {
     protected function get_site_menu_activity_element($element, $selectortype, $activityname) {
         $activitynode = $this->get_site_menu_activity_node($activityname);
 
-        // Transforming to Behat selector/locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-        $exception = new ElementNotFoundException($this->getSession(), '"' . $element . '" "' .
-            $selectortype . '" in "' . $activityname . '" ');
-
-        return $this->find($selector, $locator, $exception, $activitynode);
+        $exception = new ElementNotFoundException($this->getSession(), "'{$element}' '{$selectortype}' in '${activityname}'");
+        return $this->find($selectortype, $element, $exception, $activitynode);
     }
 
     /**
diff --git a/blocks/social_activities/tests/behat/behat_block_social_activities.php b/blocks/social_activities/tests/behat/behat_block_social_activities.php
index 2fe5526d473..8c4f836b896 100644
--- a/blocks/social_activities/tests/behat/behat_block_social_activities.php
+++ b/blocks/social_activities/tests/behat/behat_block_social_activities.php
@@ -114,12 +114,8 @@ class behat_block_social_activities extends behat_base {
     protected function get_social_block_activity_element($element, $selectortype, $activityname) {
         $activitynode = $this->get_social_block_activity_node($activityname);
 
-        // Transforming to Behat selector/locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-        $exception = new ElementNotFoundException($this->getSession(), '"' . $element . '" "' .
-            $selectortype . '" in "' . $activityname . '" ');
-
-        return $this->find($selector, $locator, $exception, $activitynode);
+        $exception = new ElementNotFoundException($this->getSession(), "'{$element}' '{$selectortype}' in '${activityname}'");
+        return $this->find($selectortype, $element, $exception, $activitynode);
     }
 
     /**
diff --git a/course/tests/behat/behat_course.php b/course/tests/behat/behat_course.php
index 35465843b41..16fa6e3e3ad 100644
--- a/course/tests/behat/behat_course.php
+++ b/course/tests/behat/behat_course.php
@@ -1144,11 +1144,8 @@ class behat_course extends behat_base {
     protected function get_activity_element($element, $selectortype, $activityname) {
         $activitynode = $this->get_activity_node($activityname);
 
-        // Transforming to Behat selector/locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-        $exception = new ElementNotFoundException($this->getSession(), '"' . $element . '" "' . $selectortype . '" in "' . $activityname . '" ');
-
-        return $this->find($selector, $locator, $exception, $activitynode);
+        $exception = new ElementNotFoundException($this->getSession(), "'{$element}' '{$selectortype}' in '${activityname}'");
+        return $this->find($selectortype, $element, $exception, $activitynode);
     }
 
     /**
diff --git a/lib/behat/behat_base.php b/lib/behat/behat_base.php
index c42d38ef5d7..c6053b9106b 100644
--- a/lib/behat/behat_base.php
+++ b/lib/behat/behat_base.php
@@ -381,12 +381,7 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return NodeElement
      */
     protected function get_selected_node($selectortype, $element) {
-
-        // Getting Mink selector and locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
-        // Returns the NodeElement.
-        return $this->find($selector, $locator);
+        return $this->find($selectortype, $element);
     }
 
     /**
@@ -398,7 +393,6 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return NodeElement
      */
     protected function get_text_selector_node($selectortype, $element) {
-
         // Getting Mink selector and locator.
         list($selector, $locator) = $this->transform_text_selector($selectortype, $element);
 
@@ -417,18 +411,13 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return NodeElement
      */
     protected function get_node_in_container($selectortype, $element, $containerselectortype, $containerelement) {
-
         // Gets the container, it will always be text based.
         $containernode = $this->get_text_selector_node($containerselectortype, $containerelement);
 
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
-        // Specific exception giving info about where can't we find the element.
         $locatorexceptionmsg = $element . '" in the "' . $containerelement. '" "' . $containerselectortype. '"';
         $exception = new ElementNotFoundException($this->getSession(), $selectortype, null, $locatorexceptionmsg);
 
-        // Looks for the requested node inside the container node.
-        return $this->find($selector, $locator, $exception, $containernode);
+        return $this->find($selectortype, $element, $exception, $containernode);
     }
 
     /**
@@ -447,7 +436,6 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * @return array Contains the selector and the locator expected by Mink.
      */
     protected function transform_selector($selectortype, $element) {
-
         // Here we don't know if an allowed text selector is being used.
         $selectors = behat_selectors::get_allowed_selectors();
         if (!isset($selectors[$selectortype])) {
@@ -511,63 +499,68 @@ class behat_base extends Behat\MinkExtension\Context\RawMinkContext {
      * Spins around an element until it exists
      *
      * @throws ExpectationException
-     * @param string $element
+     * @param string $locator
      * @param string $selectortype
      * @return void
      */
-    protected function ensure_element_exists($element, $selectortype) {
-
-        // Getting the behat selector & locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
+    protected function ensure_element_exists($locator, $selectortype) {
         // Exception if it timesout and the element is still there.
-        $msg = 'The "' . $element . '" element does not exist and should exist';
+        $msg = "The '{$locator}' element does not exist and should";
         $exception = new ExpectationException($msg, $this->getSession());
 
+        // Normalise the values in order to perform the search.
+        [
+            'selector' => $selector,
+            'locator' => $locator,
+            'container' => $container,
+        ] = $this->normalise_selector($selectortype, $locator, $this->getSession()->getPage());
+
         // It will stop spinning once the find() method returns true.
         $this->spin(
-            function($context, $args) {
-                // We don't use behat_base::find as it is already spinning.
-                if ($context->getSession()->getPage()->find($args['selector'], $args['locator'])) {
+            function() use ($selector, $locator, $container) {
+                if ($container->find($selector, $locator)) {
                     return true;
                 }
                 return false;
             },
-            array('selector' => $selector, 'locator' => $locator),
+            [],
             self::get_extended_timeout(),
             $exception,
             true
         );
-
     }
 
     /**
      * Spins until the element does not exist
      *
      * @throws ExpectationException
-     * @param string $element
+     * @param string $locator
      * @param string $selectortype
      * @return void
      */
-    protected function ensure_element_does_not_exist($element, $selectortype) {
-
-        // Getting the behat selector & locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
+    protected function ensure_element_does_not_exist($locator, $selectortype) {
         // Exception if it timesout and the element is still there.
-        $msg = 'The "' . $element . '" element exists and should not exist';
+        $msg = "The '{$locator}' element exists and should not exist";
         $exception = new ExpectationException($msg, $this->getSession());
 
+        // Normalise the values in order to perform the search.
+        [
+            'selector' => $selector,
+            'locator' => $locator,
+            'container' => $container,
+        ] = $this->normalise_selector($selectortype, $locator, $this->getSession()->getPage());
+
         // It will stop spinning once the find() method returns false.
         $this->spin(
-            function($context, $args) {
-                // We don't use behat_base::find() as we are already spinning.
-                if (!$context->getSession()->getPage()->find($args['selector'], $args['locator'])) {
-                    return true;
+            function() use ($selector, $locator, $container) {
+                if ($container->find($selector, $locator)) {
+                    return false;
                 }
-                return false;
+                return true;
             },
-            array('selector' => $selector, 'locator' => $locator),
+            // Note: We cannot use $this because the find will then be $this->find(), which leads us to a nested spin().
+            // We cannot nest spins because the outer spin times out before the inner spin completes.
+            [],
             self::get_extended_timeout(),
             $exception,
             true
diff --git a/lib/tests/behat/behat_general.php b/lib/tests/behat/behat_general.php
index 5b0e2e98de8..18bfe439365 100644
--- a/lib/tests/behat/behat_general.php
+++ b/lib/tests/behat/behat_general.php
@@ -412,24 +412,22 @@ class behat_general extends behat_base {
      * @param string $containerelement
      * @param string $containerselectortype
      */
-    public function i_drag_and_i_drop_it_in($element, $selectortype, $containerelement, $containerselectortype) {
-
-        list($sourceselector, $sourcelocator) = $this->transform_selector($selectortype, $element);
-        $sourcexpath = $this->getSession()->getSelectorsHandler()->selectorToXpath($sourceselector, $sourcelocator);
-
-        list($containerselector, $containerlocator) = $this->transform_selector($containerselectortype, $containerelement);
-        $destinationxpath = $this->getSession()->getSelectorsHandler()->selectorToXpath($containerselector, $containerlocator);
-
-        $node = $this->get_selected_node("xpath_element", $sourcexpath);
-        if (!$node->isVisible()) {
-            throw new ExpectationException('"' . $sourcexpath . '" "xpath_element" is not visible', $this->getSession());
-        }
-        $node = $this->get_selected_node("xpath_element", $destinationxpath);
-        if (!$node->isVisible()) {
-            throw new ExpectationException('"' . $destinationxpath . '" "xpath_element" is not visible', $this->getSession());
+    public function i_drag_and_i_drop_it_in($source, $sourcetype, $target, $targettype) {
+        if (!$this->running_javascript()) {
+            throw new DriverException('Drag and drop steps require javascript');
         }
 
-        $this->getSession()->getDriver()->dragTo($sourcexpath, $destinationxpath);
+        $source = $this->find($sourcetype, $source);
+        $target = $this->find($targettype, $target);
+
+        if (!$source->isVisible()) {
+            throw new ExpectationException("'{$source}' '{$sourcetype}' is not visible", $this->getSession());
+        }
+        if (!$target->isVisible()) {
+            throw new ExpectationException("'{$target}' '{$targettype}' is not visible", $this->getSession());
+        }
+
+        $this->getSession()->getDriver()->dragTo($source->getXpath(), $target->getXpath());
     }
 
     /**
@@ -968,12 +966,8 @@ EOF;
      * @param string $selectortype The selector type
      */
     public function should_exist($element, $selectortype) {
-
-        // Getting Mink selector and locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
         // Will throw an ElementNotFoundException if it does not exist.
-        $this->find($selector, $locator);
+        $this->find($selectortype, $element);
     }
 
     /**
@@ -987,35 +981,27 @@ EOF;
      * @param string $selectortype The selector type
      */
     public function should_not_exist($element, $selectortype) {
-
-        // Getting Mink selector and locator.
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
+        // Will throw an ElementNotFoundException if it does not exist, but, actually it should not exist, so we try &
+        // catch it.
         try {
-
-            // Using directly the spin method as we want a reduced timeout but there is no
-            // need for a 0.1 seconds interval because in the optimistic case we will timeout.
-            $params = array('selector' => $selector, 'locator' => $locator);
             // The exception does not really matter as we will catch it and will never "explode".
             $exception = new ElementNotFoundException($this->getSession(), $selectortype, null, $element);
 
+            // Using the spin method as we want a reduced timeout but there is no need for a 0.1 seconds interval
+            // because in the optimistic case we will timeout.
             // If all goes good it will throw an ElementNotFoundExceptionn that we will catch.
             $this->spin(
-                function($context, $args) {
-                    return $context->getSession()->getPage()->findAll($args['selector'], $args['locator']);
-                },
-                $params,
-                behat_base::get_reduced_timeout(),
-                $exception,
-                false
+                function($context, $args) use ($selectortype, $element) {
+                    return $this->find($selectortype, $element);
+                }, [], behat_base::get_reduced_timeout(), $exception, false
             );
         } catch (ElementNotFoundException $e) {
-            // It passes.
+            // We expect the element to not be found.
             return;
         }
 
-        throw new ExpectationException('The "' . $element . '" "' . $selectortype .
-                '" exists in the current page', $this->getSession());
+        // The element was found and should not have been. Throw an exception.
+        throw new ExpectationException("The '{$element}' '{$selectortype}' exists in the current page", $this->getSession());
     }
 
     /**
@@ -1159,16 +1145,14 @@ EOF;
      */
     public function should_exist_in_the($element, $selectortype, $containerelement, $containerselectortype) {
         // Get the container node.
-        $containernode = $this->get_selected_node($containerselectortype, $containerelement);
-
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
+        $containernode = $this->find($containerselectortype, $containerelement);
 
         // Specific exception giving info about where can't we find the element.
-        $locatorexceptionmsg = $element . '" in the "' . $containerelement. '" "' . $containerselectortype. '"';
+        $locatorexceptionmsg = "{$element} in the {$containerelement} {$containerselectortype}";
         $exception = new ElementNotFoundException($this->getSession(), $selectortype, null, $locatorexceptionmsg);
 
         // Looks for the requested node inside the container node.
-        $this->find($selector, $locator, $exception, $containernode);
+        $this->find($selectortype, $element, $exception, $containernode);
     }
 
     /**
@@ -1184,26 +1168,24 @@ EOF;
      * @param string $containerselectortype The container locator
      */
     public function should_not_exist_in_the($element, $selectortype, $containerelement, $containerselectortype) {
+        // Get the container node.
+        $containernode = $this->find($containerselectortype, $containerelement);
 
-        // Get the container node; here we throw an exception
-        // if the container node does not exist.
-        $containernode = $this->get_selected_node($containerselectortype, $containerelement);
-
-        list($selector, $locator) = $this->transform_selector($selectortype, $element);
-
-        // Will throw an ElementNotFoundException if it does not exist, but, actually
-        // it should not exist, so we try & catch it.
+        // Will throw an ElementNotFoundException if it does not exist, but, actually it should not exist, so we try &
+        // catch it.
         try {
-            // Would be better to use a 1 second sleep because the element should not be there,
-            // but we would need to duplicate the whole find_all() logic to do it, the benefit of
-            // changing to 1 second sleep is not significant.
-            $this->find($selector, $locator, false, $containernode, behat_base::get_reduced_timeout());
+            // Looks for the requested node inside the container node.
+            $this->find($selectortype, $element, false, $containernode, behat_base::get_reduced_timeout());
         } catch (ElementNotFoundException $e) {
-            // It passes.
+            // We expect the element to not be found.
             return;
         }
-        throw new ExpectationException('The "' . $element . '" "' . $selectortype . '" exists in the "' .
-                $containerelement . '" "' . $containerselectortype . '"', $this->getSession());
+
+        // The element was found and should not have been. Throw an exception.
+        throw new ExpectationException(
+            "The '{$element}' '{$selectortype}' exists in the '{$containerelement}' '{$containerselectortype}'",
+            $this->getSession()
+        );
     }
 
     /**
@@ -1794,8 +1776,8 @@ EOF;
         if (!$this->running_javascript()) {
             throw new DriverException('Checking focus on an element requires JavaScript');
         }
-        list($a, $b) = $this->transform_selector($nodeselectortype, $nodeelement);
-        $element = $this->find($a, $b);
+
+        $element = $this->find($nodeselectortype, $nodeelement);
         $xpath = addslashes_js($element->getXpath());
         $script = 'return (function() { return document.activeElement === document.evaluate("' . $xpath . '",
                 document, null, XPathResult.FIRST_ORDERED_NODE_TYPE, null).singleNodeValue; })(); ';

From 2920155a312dd591d06198e5b5856485cef55d28 Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Thu, 29 Aug 2019 11:37:40 +0800
Subject: [PATCH 4/5] MDL-66550 behat: Transform fields to search in a fieldset

---
 lib/behat/classes/behat_selectors.php | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lib/behat/classes/behat_selectors.php b/lib/behat/classes/behat_selectors.php
index 5f03e6c59f9..f093e759d94 100644
--- a/lib/behat/classes/behat_selectors.php
+++ b/lib/behat/classes/behat_selectors.php
@@ -27,6 +27,7 @@ require_once(__DIR__ . '/exact_named_selector.php');
 require_once(__DIR__ . '/partial_named_selector.php');
 
 use Behat\Mink\Exception\ExpectationException as ExpectationException;
+use Behat\Mink\Element\Element;
 
 /**
  * Moodle selectors manager.
@@ -94,4 +95,26 @@ class behat_selectors {
             behat_context_helper::escape($locator),
         ];
     }
+
+    /**
+     * Transform the selector for a field.
+     *
+     * @param string $label The label to find
+     * @param Element $container The container to look within
+     * @return array The selector, locator, and container to search within
+     */
+    public static function transform_find_for_field(behat_base $context, string $label, Element $container): array {
+        $hasfieldset = strpos($label, '>');
+        if (false !== $hasfieldset) {
+            [$containerlabel, $label] = explode(">", $label, 2);
+            $container = $context->find_fieldset(trim($containerlabel), $container);
+            $label = trim($label);
+        }
+
+        return [
+            'selector' => 'named_partial',
+            'locator' => self::normalise_named_selector('field', $label),
+            'container' => $container,
+        ];
+    }
 }

From bb6315f281724063c0660e0f8ceaa835b612e64c Mon Sep 17 00:00:00 2001
From: Andrew Nicols <andrew@nicols.co.uk>
Date: Thu, 29 Aug 2019 16:44:36 +0800
Subject: [PATCH 5/5] MDL-66550 forum: Target fields in a fieldset for behat
 test

---
 grade/tests/behat/grade_to_pass.feature | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/grade/tests/behat/grade_to_pass.feature b/grade/tests/behat/grade_to_pass.feature
index a59f0fc396f..e257fd95206 100644
--- a/grade/tests/behat/grade_to_pass.feature
+++ b/grade/tests/behat/grade_to_pass.feature
@@ -195,9 +195,9 @@ Feature: We can set the grade to pass value
     And I add a "Forum" to section "1" and I fill the form with:
       | Forum name     | Test Forum 1    |
       | Description    | Test               |
-      | Aggregate type | Average of ratings |
-      | Grade to pass  | 90                 |
-      | scale[modgrade_point] | 60 |
+      | Ratings > Aggregate type | Average of ratings |
+      | Ratings > scale[modgrade_point] | 60 |
+      | Grade > Grade to pass  | 90                 |
     Then I should see "The grade to pass can not be greater than the maximum possible grade 60"
 
   Scenario: Set a valid grade to pass for forum activity
@@ -205,8 +205,8 @@ Feature: We can set the grade to pass value
     And I add a "Forum" to section "1" and I fill the form with:
       | Forum name     | Test Forum 1    |
       | Description    | Test               |
-      | Aggregate type | Average of ratings |
-      | Grade to pass  | 90                 |
+      | Ratings > Aggregate type | Average of ratings |
+      | Grade > Grade to pass  | 90                 |
     And I navigate to "View > Grader report" in the course gradebook
     And I turn editing mode on
     And I click on "Edit  forum Test Forum 1" "link"