refactor(grails-geb): Read GebConfig.groovy to allow overriding RemoteWebDriver #14987
Conversation
…eWebDriver, Close apache#14801 I hereby lived under the assumption that this does not need to be as dynamic as the other grails-geb/connection settings (which can be defined per Class using Annotation/Interface), and just needs to be project-configureable. Though that may raise the question of some users why FileDetector was made configurable in this way too. This now should also open up all other gebconfig functionality. Holding out with writing Documentation until feedback on idea's was provided. Maybe I also did something in an geb-unwanted way. _Also possibly currently trying to make a diagram to better wrap my head around grails-geb's moving parts and necessities for future encounters._ <details> <summary>Unrelevant Thoughts</summary> I had originally tried to allow the use of normal Drivers and then extracting their capabilities, but as far as I was able to jungle through this is not possible. One option there would've been to intercept e.g. ChromeOptions construction when calling closure, or another hacking interception way, but i chose not to. I also had the idea to introduce a new GebConfig key, like other *-geb extensions do (e.g. grailsGeb, ). </details>
There was a problem hiding this comment.
This seems like a pretty reasonable change on first glance. @matrei has more experience with Geb so he will likely have better feedback here.
I'd want to see the documentation updated to merge this. Thank you so much for your contribution.
|
@JonasPammer the more I thought about this, we should ship a "Default" GebConfig and use it if there isn't one in the project. A lot of the projects used the same boilerplate default and that goes counter to the Grails goal of convention over configuration. Can you update the PR so that a DefaultGebConfig.groovy is shipped along side grails-geb and if one isn't specified by the end user, we use that one instead? |
What is the advantage of shipping a default |
Do a separate project for testing using default `GebConfig` file so we don't pollute the other tests.
`.tap` returns the receiver, which suggests fluent chaining. Since the return value is unused, use `.with`. Behavior is unchanged.
|
One thing one could add to his meaning may be for us to abandon our self made factories / beans and go for a gebconfig-only way to change grails-geb additions. But thats a final 7.0 decision. Also sorry for not writing this earlier, i am currently in ireland/holiday and only come back sunday for a few days. If someone wants to finish/ alter before that for RC2, all contribution takeovers are allowed and ok from my side. |
If it simplifies things without losing functionality, I'm all for it.
Hope you are having a great time in Ireland!
Thank you! I'll merge this and we discuss going all in GebConfig later. |
|
🥳❤️ |
Close #14801
I lived under the assumption that this does not need to be as dynamic as the grails-geb/connection settings (which can be defined per Class using Annotation/Interface), and just needs to be per-project configure'able. Though that may raise the question of some user why FileDetector was made configurable in this way too - If this goes through the reason here being that it was before this.
This now should also open up all other gebconfig functionality, as seen by
reportingListenerexample from official geb documentation.Holding out with writing Documentation until feedback on this idea was provided.
Maybe I also did something in an geb-unwanted way.
Unrelevant Thoughts
I had originally tried to allow the use of normal Drivers and then extracting their capabilities, but as far as I was able to jungle through this is not possible. One option there would've been to intercept e.g. ChromeOptions construction when calling closure, or another hacking interception way, but i chose not to.I also had the idea to introduce a new GebConfig key, like other *-geb extensions do (e.g. grailsGeb, ).
Also I may possibly currently be trying to make a diagram to better wrap my head around grails-geb's moving parts and necessities for future encounters.