Allow accessing selectors handler in Session

[aik099]

In the 1.x versions it was possible to use Session::getSelectorsHandler() method to get selectors handler and change it (e.g. add new selectors to it).

In the 2-architecture-changes branch things have changed considerably and now we have (see https://github.com/Behat/Mink/blob/2-architecture-changes/src/Behat/Mink/Session.php#L46):

(DriverInterface + SelectorsHandler + Manipulator) = ElementFinder
(DriverInterface + ElementFinder) = Session

As you can see the SelectorsHandler is now created deep within ElementFinder and ElementFinder itself is created within Session.

If developer has full control over Session creation then of course he can create ElementFinder as he see fits and in the process obtain reference to SelectorsHandler.

However within Behat developer gets already created session via ->getSession() method that he can't change anymore.

[aik099]

Also a strange thing is that Session internally creates ElementFinder that immediately goes to the DocumentElement that is created and then never uses ElementFinder at all.

What I'm saying here is that Session needs Driver and DocumentElement to operate, but not Driver and ElementFinder that give to it.

[stof]

Within Behat, you should use an extension to register more selectors in the selectors handler IMO.

What I'm saying here is that Session needs Driver and DocumentElement to operate, but not Driver and ElementFinder that give to it.

The difference is that the ElementFinder is a service which can be defined externally to be configured. On the other hand, it does not reallyt make sense for a Session to receive a DocumentElement created externally IMO

[aik099]

Within Behat, you should use an extension to register more selectors in the selectors handler IMO.

Is there any example on how to do it?

[aik099]

Within my library I've creates sub-class from NodeElement, but since Mink returns only NodeElements I need to create fromNodeElement static method, that created by sub-class with xpath and session used in NodeElement: https://github.com/qa-tools/qa-tools/blob/develop/library/QATools/QATools/PageObject/Element/WebElement.php#L74

Problem that I'll face if I want to upgrade to Mink 2.0 is that NodeElement there doesn't have any public getters that I can use to obtain it's dependencies (driver & element finder).

[stof]

@aik099 why using inheritance on the NodeElement ? It may be better to build your own API by composition, by wrapping the NodeElement in your own class

[aik099]

If I wrap it, then to provide access to all NodeElement methods I need to do either:

1. create __call method and matching @method records in class DocBlock
2. create as much methods as NodeElement has, that will only forward calls to NodeElement

If I go 1st way, then I might get overhead due __call method usage. If I go the 2nd way, then I'll have very strange looking class.

[stof]

third solution: give access to the underlying NodeElement

Btw, your current API is already weird: you redefine waitFor with a different signature than in Mink

[aik099]

you redefine waitFor with a different signature than in Mink

Because it makes sense to wait in seconds, rather then milliseconds. I haven't seen any potential usage for such small waiting timeouts (except for in JavaScript), so I raised it to seconds (from milliseconds). Also it makes more sense to specify 0.5sec instead of 500ms to me.

[aik099]

third solution: give access to the underlying NodeElement

That will complicate things when dealing with AbstractTypifiedElement class (see https://github.com/qa-tools/qa-tools/blob/master/library/QATools/QATools/HtmlElements/Element/AbstractTypifiedElement.php), because it has getWrappedElement that will return WebElement around which it is wrapped. If we expose used NodeElement from WebElement, then accessing it from AbstractTypifiedElement will be madness: AbstractTypifiedElement -> WebElement -> NodeElement.

[aik099]

Also WebElement, that extends NodeElement perfectly represents similary named class in Selenium itself. There WebElement is interface which is implemented by anything that can be found by Selenium.

[stof]

Because it makes sense to wait in seconds, rather then milliseconds.

yeah, but some code expecting a NodeElement would then be unable to use your own NodeElement, becaus eof a different meaning for the argument it passes. Your WebElement breaks the OOP rule (child classes should be usable in all places expecting the parent class)

then accessing it from AbstractTypifiedElement will be madness

AbstractTypifiedElement could simply add a shortcut proxying the call if needed

[aik099]

Your WebElement breaks the OOP rule (child classes should be usable in all places expecting the parent class)

And it is usable. Child class also is able to change the way how methods of parent class work and that's normal. Say we have Car class with drive($meters) method (that takes X$meters seconds to run) and it's sub-class SportsCar, which has drive($meters) method (that takes Y$meters seconds to run). That doesn't break anything.

If I keep milliseconds, then it would create inconsistency in other library parts, where waiting is done is seconds. I originally created waitFor in Mink with milliseconds just because wait method was using milliseconds. Maybe for waitFor the seconds makes more sense also for Mink, since it's waiting for element, not for some unknown JS action to happen as wait method does?

AbstractTypifiedElement could simply add a shortcut proxying the call if needed

It does so already for some common methods, but having shortcuts for all methods just doesn't make sense considering whole TypifiedElement idea. NodeElement for example has all kinds of methods (27 methods) and even ones, that doesn't make sense to have on found element (e.g. check on a div element). The AbstractTypifiedElement has only common methods for all element types. Then sub-class from AbstractTypifiedElement exists (e.g. Checkbox, Button, etc.), that adds only relevant methods to it.

So I already use the composition idea you've proposed within the AbstractTypifiedElement class. Problem is, that creating double composition would require me to create 27 public methods of NodeElement within WebElement to make composition work.

[stof]

And it is usable. Child class also is able to change the way how methods of parent class work and that's normal

you are not changing how it works. you are changing the API. And this is not normal

It does so already for some common methods

I was suggesting to proxy getNodeElement, not all methods of NodeElement

[aik099]

you are not changing how it works. you are changing the API. And this is not normal

How to detect what's being changed? To me public API is names/types of method and it's arguments and return value type. That is kept the same. What method does internally isn't relevant, isn't it?

I was suggesting to proxy getNodeElement, not all methods of NodeElement

I understood that already, but it will look like hell from TypifiedElement perspective: $checkbox->getWrappedElement()->getNodeElement()->click(). Also WebElement meant as extension to the NodeElement so anybody who is used to work with NodeElement can migrate easily. The method that seams wrong is waitFor, but as I advised then maybe it's Mink's method version that should be changed.

[stof]

How to detect what's being changed? To me public API is names of method and it's arguments and return value type. That is kept the same. What method does internally isn't relevant, isn't it?

The public API says that the duration is an integer representing milliseconds. You are now sayign it is seconds. So any code written against the Mink API will wait 1000 times longer when receiving your own object

[aik099]

Ok, so I ask 3rd time: how about changing it to seconds also in Mink (in waitFor method only, that isn't released yet)?

[aik099]

If you agree, then I've already created PR for that: #603

[stof]

@aik099 coming back to the original topic, what is the actual use case for accessing the SelectorsHandler from the Session in 2.0 ?

[aik099]

Now with #603 PR merged and waitFor method removed from WebElement I'm no longer breaking public API so it's OK to keep extending NodeElement I guess.

Here are the use cases:

• to use xpathLiteral method (passing it in element class constructor will only complicate manual element creation and instantiating it in-place is bad practice)
  • Form class
  • LabeledElement class
  • Select class
• adding new se selector to existing SelectorsHandler in https://github.com/qa-tools/qa-tools/blob/develop/library/QATools/QATools/PageObject/Element/WebElement.php#L71 so that selector building logic can be kept in separate class
• the se is then used either for xpath creation or find method calls on NodeElement descendant classes

I wonder how Behat is able to inject new selectors in existing SelectorsHandler even before it goes into Session. Maybe with current API where SelectorsHandler is injected into Session this isn't a problem.

[stof]

to use xpathLiteral method

method is gone in 2.0 (the Escaper class of 1.6+ should be used, or nothing at all for the named selector as it now escapes the locator properly)

adding new se selector to existing SelectorsHandler

Can you link to the place in QATools doing this registration ?

the se is then used either for xpath creation or find method calls on NodeElement descendant classes

hmm, the need to convert selectors to XPath separately from the Mink element finding is indeed a case where it would be needed to access it. Is your WebElement class expected to be instantiated by the user or could it be done from a place knowing the SelectorsHandler separately from the session ?

For find methods, you don't need the selectors handler (the find method is using it internally).

I wonder how Behat is able to inject new selectors in existing SelectorsHandler even before it goes into Session. Maybe with current API where SelectorsHandler is injected into Session this isn't a problem.

Behat relies on dependency injecting (using the Symfony component). To register your own selector, you would define it as a service in a Behat extension and tag it appropriately so that it is found by MinkExtension: https://github.com/Behat/MinkExtension/blob/2ab8c00c59995824c6ebcd50617f8c3a4d78d2d9/src/Behat/MinkExtension/ServiceContainer/MinkExtension.php#L286

[aik099]

method is gone in 2.0 (the Escaper class of 1.6+ should be used, or nothing at all for the named selector as it now escapes the locator properly)

Now is since when exactly, the Mink 1.6 release? I though that auto-escaping in NamedSelector was only done in 2.0 version.

Can you link to the place in QATools doing this registration ?

https://github.com/qa-tools/qa-tools/blob/develop/library/QATools/QATools/PageObject/PageFactory.php#L192-L194 - the method setSession is called from PageFactory class constructor as each PageFactory instance is bound to Mink's Session it was created for. This way in the RawMinkContext descendant class in Behat or BrowserTestCase descendant in PHPUnit-Mink we can do new PageFactory($this->getSession()) and that's it.

Is your WebElement class expected to be instantiated by the user or could it be done from a place knowing the SelectorsHandler separately from the session ?

Usually it's instantiated automatically, but all element classes (including typified ones, which wrap around WebElements) can be instantiated manually and I don't want to make it harder for user to do it because of additional dependencies in constructor.

Since in Mink 1.6 you can create NodeElement only by providing xpath and session then this is how you can create WebElement also. Since PageFactory already has session object inside it has no problems creating more elements with that session.

For find methods, you don't need the selectors handler (the find method is using it internally).

I'm not using named or any other build-in selectors handler so the only way to use my own is to register it. That's what I'm doing with se name.

[stof]

Now is since when exactly, the Mink 1.6 release? I though that auto-escaping in NamedSelector was only done in 2.0 version.

No, now is the 2.0 branch (this issue talks about 2.0)

[aik099]

https://github.com/minkphp/Mink/issues/602#issuecomment-54876073

I've created https://github.com/qa-tools/qa-tools/issues/168 to use composition in WebElement. Not sure if I'll end up in creating BC break in QA-Tools by doing so.