[rb] support w3c values in options class

[titusfortner]

Right now, using multiple custom configurations with Ruby is difficult.

Examples here:
https://gist.github.com/titusfortner/d40a030551be98000912f159c639c14c

This takes the current code one step further. When serializing an Options class instance,

1. Process known w3c capabilities
2. Process any capabilities passed in with a : in them (as required by spec for w3c compliant add-ons.
3. Process everything else as a browser-specific option.

If you want to keep doing it the way you are doing it, you can, but I like having the option of doing everything in one class. This makes it much easier to pull in capabilities from a yaml, which is my preference.

I couldn't figure out a better way to reference the browser name and access it everywhere I needed, but this works.

[p0deje]

It feels off to me that we've been decoupling a bloated capabilities class into capabilities/options and now we're basically stepping back and making options class bloated again. We're essentially making options classes behave like capabilities class used to be - a single place that accepts everything.

It also looks strange to me to see that Chrome::Options object can accept browserName - it seems to violate the whole purpose of browser-specific options classes.

I don't see any problem with following the existing approach which makes a separation clear:

1. Capabilities class is responsible for global (W3C) options.
2. Options class is responsbile for browser-specific options.

The only thing that doesn't make sense here is Sauce options because it doesn't fit into any of the mentioned classes. It's not global but it's neither browser-specific. So what if allow passing multiple options?

chrome_options = Selenium::WebDriver::Chrome::Options.new(args: ['--disable-infobars'])
sauce_options = {name: "Test Name"} # it can also be Sauce::Options object responding to #as_json provided by some gem
capabilities = Selenium::WebDriver::Remote::Capabilities.chrome(platform_name: 'Windows 10')
url = 'https://ondemand.saucelabs.com:443/wd/hub'

driver = Selenium::WebDriver.for(:remote, url: url, desired_capabilities: capabilities, options: [chrome_options, sauce_options])

I can imagine it should also fit it into the YAML configurations just fine. Can you please provide more example of those for better context?

[titusfortner]

we've been decoupling a bloated capabilities class into capabilities/options and now we're basically stepping back and making options class bloated again.

I've never liked the Capabilities implementation. The way we did some things like Ruby methods and some things like Hashes, and the different behaviors between string keys and symbol keys, and differences between what was serialized and deserialized. Part of this was because there wasn't a published spec that allows you to reject non-standard things, but overall this is my proposal for how to make the code design more sane.

We're essentially making options classes behave like capabilities class used to be - a single place that accepts everything

Except, this is what both Java & C# have done. C# doesn't allow you to do anything with Capabilities any more, it *has to be Options, and Java has made it so that to do multiple configurations everything gets put into a MutableCapabilities class. This just allows Ruby to behave like those other bindings.

It also looks strange to me to see that Chrome::Options object can accept browserName - it seems to violate the whole purpose of browser-specific options classes.

Yeah, it makes sense to pull that out.

what if allow passing multiple options?

@twalpole when we were working together in February, I remember we looked into this approach. I think I later decided I didn't like it, but I don't remember why. Did I decide I didn't like it that weekend, or was it after that? Do you remember?

[twalpole]

I don't remember why you didn't like the multiple options approach either.

The big issue here is that no one knows where any specific setting needs to be made and end up just trying to push raw hashes into everywhere until things work. Then we end up with issues like lots of people adding "chromeOptions" => { ...} to capabilities that silently stop working when chromedriver swaps to "goog:chromeOptions".

I also think "desired_capabilities" is a bad name for this - does it mean I will get what I request, or does it mean I might get it but have to check to be sure? What is a capability vs an option ?? There's just no clear definition of what's what from a user perspective

[titusfortner]

Naming is hard. It isn't obvious to a user what would go in capabilities vs options, or that an options parameter would take either a class or an array of classes and/or hashes. I want more simplicity ("just put everything here"), or at least an option for it.

Ideally I would like to prevent (entirely) users from doing the (currently required) free-form / write your own Hash from scratch to initialize sessions. That shouldn't be how we want people to write Ruby. We know everything we need to in order to build everything that people need to create a session in a safe way that provides the right structure and gives the right error messages.

If we aren't going to do that (by getting rid of the direct usage of the Capabilities class), I'd like to at least compromise and provide a protected way to put all of the information used to create a session into one class that does the one thing -- build the session payload.

[p0deje]

Except, this is what both Java & C# have done. C# doesn't allow you to do anything with Capabilities any more, it *has to be Options, and Java has made it so that to do multiple configurations everything gets put into a MutableCapabilities class. This just allows Ruby to behave like those other bindings.

You're right. I don't like that there is a blurry line between capabilities and options in Java and C# implementations - I also don't know the reason behind mixing them. In my opinion, the implementation should look like this:

1. Capabilities class is only responsible for spec new session payload (browserName, platformName, alwaysMatch, etc.) and anything else the user wants to pass as raw payload. The latter should probably be discouraged as in the example with chromeOptions Thomas gave. But it should be possible. Anything that is in Capabilities class should be supported by all remote implementations.
2. Options class is only responsible for browser-specific options. Thus, no browserName, platformName and others - it's all irrelevant from the browser point of view. Most of the options are serialized into a simple hash that is merged with capabilities internally. However, some options classes might do something more complicated as long as it is browser-specific (e.g. Chrome::Options should both represent goog:chromeOptions and goog:loggingPrefs). Anything that is in Options class should be supported by at least one remote implementation.

I also don't believe we really need to use one class to build session payload - the mixed usage for capabilities + options sounds reasonable to me. It is not so simple, but it is easy.

I see the value in making the implementation consistent across the bindings even though I don't necessarily like it. I also see how having a single class can be more convenient though I believe that higher-level wrapper like Watir/Capybara can provide even more convenient classes for setting options. I also see the value that having 1 class makes things more simple for a user, even though it's kind of allowed already via capabilities class.

[titusfortner]

It is not so simple, but it is easy.

I vastly prefer simple to easy. Almost every problem I've come across in code / test framework design can be partially attributed to someone complicating things by trying to make them easier (I've been guilty of this as well), and making them harder to maintain/debug.

The problem with your 2 pieces there is, what do we do with additional/custom selenium options, and what do we do with additional 3rd party things like Sauce? If everything fit obviously into those 2 pieces, then I wouldn't be as concerned.

The problem with the Capabilities class right now is that it allows too many things in too many different ways. The main reason I stepped away from doing Selenium development 2 years ago is when I was trying to figure out how to test everything that class was doing and make sure all the ways that it let you do things worked as intended. It was/is a nightmare to make sure it is correct, and it encourages a lot of bad practices imo. Now that we have consistent settings and documentation we can make it so much better.

[p0deje]

I might misunderstand simple vs. easy. Please, correct me if I'm wrong:

• Passing raw capability is simple but is not easy because you don't know what exactly to send. One has to google capability name and value.
• Using options API might not be so simple (you probably need to write a bit more code), but is easy because the editor autocompletes possible methods and their values.

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[titusfortner]

closed in favor of #7832