Mink element selection improvements

[uuf6429]

As mentioned here, eventually I ended up with an improved, (for dev point of view), element selection system on top of Mink.

Let's take a look first at how it's currently working. Let's say we want to check the result of a form submission and in particular we expect a specific text in a specific element (to avoid conflicting with other elements on the same page that may have the same text).

$session->visit('https://example.com/login');
$page = $session->getPage();

// <fill and submit form>

$welcomeDiv = $page->find('css', '.welcome-message');
assert(str_contains($welcomeDiv->getText(), 'Welcome, testuser'));

Mink provides quite a few methods that take selector and locator arguments. While that's not wrong, it is does have a certain learning curve. Since the documentation is a bit lacking, devs will probably have to dig a bit to find the posibilities.

What I'm proposing is to have a specific data structure (class) encapsulating that selector/locator pair.

Interestingly, the code in Selector/ seems to lead in that direction, but seems to fall short of being useful to end users.

---

Here's a quick backward-compatible prototype of what I mean:

interface ElementInterface  // methods below are added to the existing interface
{
    public function hasElement(ElementSelectorInterface $selector): bool;
    public function findElement(ElementSelectorInterface $selector): null|NodeElement;
    /**
     * @return list<NodeElement>
     */
    public function findElements(ElementSelectorInterface $selector): array;
}

class Element // methods below are added to the existing class
{
    public function hasElement(ElementSelectorInterface $selector): bool
    {
        return $this->has($selector->getType(), $selector->getLocator());
    }

    public function findElement(ElementSelectorInterface $selector): null|NodeElement
    {
        return $this->find($selector->getType(), $selector->getLocator());
    }

    public function findElements(ElementSelectorInterface $selector): array
    {
        return $this->findAll($selector->getType(), $selector->getLocator());
    }
}

namespace Selectors;

interface SelectorInterface
{
    public function getType(): string;   // In mink, this is actually known as the "selector", but
                                         // I renamed it to avoid confusion with the interface name.

    public function getLocator(): string|array;
}

class AbstractSelector implements SelectorInterface
{
    public function __construct(
        private readonly string $type,
        private readonly string|array $locator,
    ){}

    public function getType(): string { return $this->type; }

    public function getLocator(): string|array { return $this->locator; }
}

class Css extends AbstractSelector
{
    public function __construct(
        #[Language('CSS')]
        string $selector,
    ) {
        parent::__construct('css', $selector);
    }
}

class ExactName extends AbstractSelector
{
    /**
     * @param 'fieldset'|'field'|'link'|'button'|'etc' $nameType
     */
    public function __construct(string $nameType, string $nameValue)
    {
        parent::__construct('named', [$nameType, $nameValue]);
    }
}

class Name extends ExactName
{
    public function __construct(string $fieldName)
    {
        parent::__construct('name', $fieldName);
    }
}

class Field extends ExactName
{
    public function __construct(string $fieldName)
    {
        parent::__construct('field', $fieldName);
    }
}

I'd consider adding equivalent methods to get visible elements (i.e. the element must be found and it must be visible), since in most cases, that's what testers intended and also decreases a lot of boilerplate.

voilà:

Some remaining points/questions

1. Does this mean building the entire object/class hierarchy for each named selector?
   Not sure. It does sound a bit far fetched.
2. What about the current mink convenience methods (e.g. $page->fillField(..)).
   While the above provides better DX (than the current selector/locator system), the conviniece methods may make more sense. I think we should keep them; at most maybe refactor them to use the new system.
3. What about other IDEs?
   I believe that anything that doesn't impact the performance of the project and that would improve DX should be added. I'm not familiar with alternatives, but if, for example, there would be a @param-language phpdoc for some specific IDE, I'd be fine adding them.

[aik099]

It's not as bad as it seems. There is a documentation page showing all possible selector/locator values: https://mink.behat.org/en/latest/guides/traversing-pages.html#selectors.

Architecturally, the idea of having \Behat\Mink\Element\Element::find method (and whoever calls it) accept $locator which has different type based on the $selector (the string in case $selector={xpath, css} and array in case $selector={named}) smells. Adding \Behat\Mink\Element\Element::findElement method, with a better signature, next to it makes it smell even worse.

Perhaps, as a BC break, we could modify the signature of the \Behat\Mink\Element\Element::find method from string $selector, $locator to ElementSelectorInterface $selector.

Theoretically, we can adjust PhpStorm auto-complete (via DocBlock or .phpstorm.meta.php file) to suggest values for $selector argument as {xpath, named, named_partial, named_exact, css}. That should help discover selectors. But that won't account for any custom-made selectors.

I'm not sure if using these intermediate classes to define a selector+locator combo would result in any overhead.

How I use Mink

• don't use any of the convenience methods because they're locked to the named selector with limited customization abilities;
• prefer using css or xpath selectors instead of named', array('field', ...) because they put me in control;
• writing $page->findElement(new Css(...)) instead of $page->find('css', ...) doesn’t offer any real advantages.

TODO

1. You're using ElementSelectorInterface, but declaring it as SelectorInterface (which already exists in Mink). Please use any of them, but consistently.
2. The find the posibilities link references \Behat\Mink\Selector\NamedSelector::$replacements property, but I guess you've meant \Behat\Mink\Selector\NamedSelector::$selectors property.
3. you're using PHP 8.1 syntax (like Readonly Properties and Union Types which needs to be changed if you plan for this to work on PHP 7.2).

Questions

1. The Selenium-based drivers already return only visible elements, but other doesn't have such a concept. How do you plan to implement getVisibleElement method idea?

List of the convenience methods (all are powered by the named selector):

For anyone using Mink directly (not through the Behat/QA-Tools/etc. library), they might provide a good starting point.

• \Behat\Mink\Element\TraversableElement::findById (does $this->find('named', array('id', $id)))
• \Behat\Mink\Element\TraversableElement::findLink (does $this->find('named', array('link', $locator)))
  • \Behat\Mink\Element\TraversableElement::hasLink
  • \Behat\Mink\Element\TraversableElement::clickLink
• \Behat\Mink\Element\TraversableElement::findButton (does $this->find('named', array('button', $locator)))
  • \Behat\Mink\Element\TraversableElement::hasButton
  • \Behat\Mink\Element\TraversableElement::pressButton
• \Behat\Mink\Element\TraversableElement::findField (does $this->find('named', array('field', $locator)))
  • \Behat\Mink\Element\TraversableElement::attachFileToField
  • \Behat\Mink\Element\TraversableElement::checkField
  • \Behat\Mink\Element\TraversableElement::fillField
  • \Behat\Mink\Element\TraversableElement::hasCheckedField
  • \Behat\Mink\Element\TraversableElement::hasField
  • \Behat\Mink\Element\TraversableElement::hasUncheckedField
  • \Behat\Mink\Element\TraversableElement::selectFieldOption
  • \Behat\Mink\Element\TraversableElement::uncheckField
  • \Behat\Mink\WebAssert::fieldExists
  • \Behat\Mink\WebAssert::fieldNotExists
• \Behat\Mink\Element\TraversableElement::hasSelect (does $this->has('named', array('select', $locator)))
• \Behat\Mink\Element\TraversableElement::hasTable (does $this->has('named', array('table', $locator)))

[uuf6429]

@aik099 thanks, glad to see that the idea is well received.

It's not as bad as it seems.

Agreed, but the current approach does require us to keep it in sync with documentation outside of code. I for one wasn't even aware of the documentation you linked to. 😅 In short: it would be nicer to write code that is self-documenting. In this case, stricter types (class/interface instead of strings) would narrow things down.

Adding \Behat\Mink\Element\Element::findElement method, with a better signature, next to it makes it smell even worse.

1. That would be to keep it backward compatible. We can deprecate and then remove find().
2. Additionally, I think that some of the current methods are ambiguously named, for example has() actually checks the element's children, not the element itself (compare with javascript's matches)
3. That's also the reason why progress in behat has been extremely slow - we're too worried about the current situation to actually move on. (that's just my opinion though)

Theoretically, we can adjust PhpStorm auto-complete (via DocBlock or .phpstorm.meta.php file) to suggest values for $selector argument as {xpath, named, named_partial, named_exact, css}

I think that would be extremely difficult and next to impossible for certain things. The final result would be to say that "when $selector is 'css' then $locator must be a string containing a css selector".. I don't see why we should choose bad design and a tonne of hacks, over some simple classes that would easily do the job.

I'm not sure if using these intermediate classes to define a selector+locator combo would result in any overhead.

Pretty sure that would be negligible and not at all a good reason considering the many benefits.

don't use any of the convenience methods because they're locked to the named selector with limited customization abilities;

Strictly speaking, good tests should go by well-defined locators e.g. element names and roles instead of e.g. css classnames, so I see why we would want to keep those methods. But I also see how css selectors can work in both cases. I think I would keep them as they are.

writing $page->findElement(new Css(...)) instead of $page->find('css', ...) doesn’t offer any real advantages.

In general, I disagree: I see a lot of advantages, actually:

1. users might want to go for an element library (again, something that we have in my case), to avoid having duplicate selectors all over the place.

   $this->findElement($library->get('Login form > username'));

2. When defining the selectors, there could be syntax highlighting that:
   1. would display the correctness of the syntax
   2. allow navigation to elements by the said selectors (e.g. ctrl+click'ing inside the selector)
3. Classes might be extended by users to perform all sort of interesting things. For example, something that I just came up with now:

   class RandomRowSelector implements SelectorInterface
   {
       public function __construct(private readonly string $tableName, private readonly $page) {}
   
       public function getType(): string { return 'css'; }
   
       public function getLocator(): string|array {
           $count = $this->page->evaluateScript(
               "return document.querySelector('table[name=\"{$this->tableName}\"]').rows.length;"
           );
           $index = mt_rand(1, $count);
           return "table[name=\"{$this->tableName}\"] tr:nth-child({$index})";
       }
   }

You're using ElementSelectorInterface, but declaring it as SelectorInterface (which already exists in Mink). Please use any of them, but consistently.

The code I posted was just to give an idea. The names area are not final. The bigger question here is if/how these classes would relate to mink's selector classes.

The find the posibilities link references \Behat\Mink\Selector\NamedSelector::$replacements property, but I guess you've meant \Behat\Mink\Selector\NamedSelector::$selectors property.

True, that was the intention. I'll update that.

you're using PHP 8.1 syntax (like Readonly Properties and Union Types which needs to be changed if you plan for this to work on PHP 7.2).

Yep, as said, just a quick prototype. The exact syntax would depending on which version we decide to target.

How do you plan to implement getVisibleElement method idea?

I haven't thought too much about it. I guess that for real web browsers, this can always be implemented using the javascript visibility api. For other cases, that would depend on the driver. A driver simulating JSDOM/CSSOM might also be able to simulate visibility. Drivers interacting with plain html might simply return "true" in all cases.

[aik099]

We can advance this even further.

Current state

• there is a \Behat\Mink\Selector\SelectorInterface class with translateToXPath($locator) method
• there are 4 classes implementing this interface:
  • \Behat\Mink\Selector\CssSelector
  • \Behat\Mink\Selector\NamedSelector
  • \Behat\Mink\Selector\ExactNamedSelector (subclass of NamedSelector)
  • \Behat\Mink\Selector\PartialNamedSelector (subclass of NamedSelector)
• there is a \Behat\Mink\Selector\SelectorsHandler class, where 3 of 4 above-listed classes are registered with an alias:
  • $this->registerSelector('named_partial', new PartialNamedSelector());
  • $this->registerSelector('named_exact', new ExactNamedSelector());
  • $this->registerSelector('css', new CssSelector());
• the \Behat\Mink\Element\ElementFinder::findAll method checks if $selector=named and tries to use named_exact and then named_partial
• a developer could also define own \Behat\Mink\Selector\SelectorInterface implementing classes and register them in the \Behat\Mink\Selector\SelectorsHandler class instance.

@uuf6429 proposed idea

Have a parallel class tree where we declare a class for each $selector/$locator combination given to the find (findElement) method.

Proposed idea modification:

Why don't we combine these class trees:

1. create lots of classes that will contain only a fraction of ExactNamedSelector, PartialNamedSelector classes (e.g. FieldSelector, PartialFieldSelector, LinkSelector, PartialLinkSelector)
2. create XPathSelector class (just to be consistent)
3. the CssSelector stay as-is
4. all of these selector classes:
   • in their constructor would accept an optional $locator (name depends on selector) argument (it's optional to preserve BC)
   • would have getXPath method (likely need to add it to the \Behat\Mink\Selector\SelectorInterface), that will return $this->translateToXPath($this->locator)
5. the NamedSelector, ExactNamedSelector and PartialNamedSelector classes would instantiate PartialLinkSelector and similar classes internally to produce a desired xpath

Unknowns

• Not sure through if we don't end up in lots of xpath fragment copy/pasted code that is now centralized in the \Behat\Mink\Selector\NamedSelector::$replacements property.
• What would happen with \Behat\Mink\Selector\NamedSelector::registerNamedXpath and \Behat\Mink\Selector\NamedSelector::registerReplacement methods?

[aik099]

Update 1:

1. Instead of having FieldSelector($locator) and PartialFieldSelector($locator) we can have FieldSelector($locator, $partial = false) instead. That would reduce added selector classes count twice.
2. Not sure how to implement named_exact -> named_partial approach from the \Behat\Mink\Element\ElementFinder::findAll method so that conventional methods could benefit from this as well
3. I'm using xpath and css selectors only inside PageObject classes. This way in test classes developers aren't even interacting with Mink directly (like in Behat).

[stof]

The behavior we have in ElementFinder::findAll cannot be replicated in a class that is only responsible for providing the XPath to find an element, as the behavior involves actually running the search and trying another one if the first search found no results.

The only idea I could see would be that the selector interface (or a separate child interface) has a method getFallback(): ?ElementSelectorInterface (if it is separate, we might not need it to be nullable, maybe), that ElementFinder would use.

2. so that conventional methods could benefit from this as well

Conventional methods do benefit from the behavior of named today, as they still use the ElementFinder to find elements.