Mink icon indicating copy to clipboard operation
Mink copied to clipboard

Move declaration of replacements and selectors to new function

Open andrewnicols opened this issue 3 years ago • 2 comments

Move declaration of replacements and selectors to new function

At present the declaration of all raw un-processed selectors and their replacements is in either the class body (private vars) or the constructor.

This has the effect of modifying the original selectors on the class instance vars during class instantiation, and also prevents any subclass of ExactNamedSelector or PartialNamedSelector from making changes to any replacement that has been explicitly overridden in those classes constructors.

For example, it is impossible for a child class of either of these to further override the '%tagTextMatch%' replacement because those changes would either be overridden in the constructor of its parent, or all original selector replacements would not be applied.

class MyPartialNamedSelector extends PartialNamedSelector
{

    public function __construct()
    {
        // Note: Calling the constructor here (before registerReplacement)
        // will mean that the call to registerReplacement will update the
        // replacement, but only for any new selector defined after the
        // replacement is defined.
        // It will not have any effect upon any selector or replacement
        // already registered.
        parent::__construct();

        $this->registerReplacement('%tagTextMatch%', 'contains(text(), %locator%)');

        // Note: Calling the constructor here (after registerReplacement) will
        // cause the above tagTextMatch replacement to be overwritten by the
        // override defined in the constructor of PartialNamedSelector.
        parent::__construct();
    }
}

Furthermore the original values of both the selectors and replacements have been mutated in the constructor so it is not possible for any child class to simply re-apply the original translations.

The only solution to this problem is to extend the NamedSelector base class and manually copy the standard replacements from PartialNamedSelector or ExactNamedSelector (as appropriate) into your new child class, but this approach is not sustainable.

This patch modifies the Selector classes to:

  • moves the storage and fetching of the original, unmodified and untranslated selectors and replacements to a function. This effectively makes them immutable; and
  • allows the child classes to override or define their own selectors and replacements by simply overriding the parent class function and updating or adding the required array values.

This change allows the replacements (or selectors) to be updated as in the following example:

class MyPartialNamedSelector extends PartialNamedSelector
{
    protected function getRawReplacements()
    {
        return array_merge(parent::getRawReplacements(), [
            '%tagTextMatch%' => 'contains(text(), %locator%)',
        ]);
    }

    protected function getRawSelectors()
    {
        return array_merge(parent::getRawSelectors(), [
            'link' => './/a[./@href][%tagTextMatch%]',
        ]);
    }
}

Note: For any usage where the child class was directly extending the NamedSelector class and defining its custom selectors and replacements in the constructor, these will need to be updated to define the relevant getRawSelectors() and/or getRawReplacements() classes as above.

This change should be considered a possibly breaking change in this situation and therefore would demand a new major release (1.10.0 presumably).

Whilst this is a breaking change, it will have provide a more sustainable approach in future for more advanced uses of Mink.

andrewnicols avatar Oct 14 '21 00:10 andrewnicols

Note: I have tried to determine how a unit test could be written here, but cannot think of a good way which does not use either a set fixture, or eval() to define a new class. If either of tese is appropriate then I can look to write some unit tests for it.

andrewnicols avatar Oct 14 '21 00:10 andrewnicols

Note: I have tried to determine how a unit test could be written here, but cannot think of a good way which does not use either a set fixture, or eval() to define a new class. If either of tese is appropriate then I can look to write some unit tests for it.

@andrewnicols , you can test this by defining a new selector sub-class with replacements, that:

  1. enhance existing ones to make them find more stuff
  2. add new replacements that are checked via new tests

aik099 avatar Oct 14 '21 18:10 aik099