Checks for disabled select/option when using selectOption* methods

[bereg2k]

Changes

Before:

SelenideElement.selectOption* methods used underlying Selenium's Select.setSelected() method that simply clicked on the option. If <select> element was disabled or the selected <option> was disabled , no exceptions were raised, and the test continued.

Now:

All SelenideElement.selectOption* methods will throw InvalidStateException if the test selects an option either from disabled <select> element or the <option> itself is disabled.

[asolntsev]

@bereg2k The implementation looks good.
I am in doubt because of 2 things:

1. These additional checks will make tests slower (every call to isEnabled etc. executes additional http request)
   (though, the current implementation already executes quite a lot of http requests).
2. Now Selenide code contains some lines copied from Selenium projects (all these xpaths for searching options by index/texts) which may be changed on Selenium side at any moment. By the way, they are already different (see select.selectByVisibleText implementation).

NB! Probably we should just implement the whole Select logic in JavaScript? Then we don't bother about performance, and can use extendable implementation for supporting all those Bootstrap, Vue.js and other frameworks (which have own select implementations).

[bereg2k]

@asolntsev

1. I checked the metrics on the checks of the recent pull request that doesn't add anything similar under the hood, and they are no different in the execution time. See Enhance closest() and ancestor() methods to search by attribute and attribute w/ value #1554 Is it really that slow to bother?
2. Selenium version 3 had no updates in years. And doesn't Selenide project control the version of Selenium library that it uses via its dependencies? It can stay on the older version as long as needed to implement any additional changes in changed (by Selenium) methods.

[bereg2k]

Regarding JS implementation.
Will it solve the issue with checking for enabled/disabled states too? My JS skills are pretty basic to get it.

[asolntsev]

@bereg2k After a long delay, I returned back to this PR.
I tried to implement these checks with JavaScript - it was not trivial. Probably I will finish it some day, but not know.

But I realised that the check for disabled should be implemented on Selenium side, right?
I prepared a PR for Selenium, let's see if they accept it: SeleniumHQ/selenium#10814

Anyway, I am going to merge your PR - at least to have these tests in our test suite.

[bereg2k]

@asolntsev I think it was a great idea to implement these checks on the Selenium side.
However, we are at crossroads now:

1. If your CR is approved and merged then my MR is irrelevant. All the exceptions will be thrown by Selenium. Selenide's exception throwing will never be reached. The unit tests in this MR will even fail because your implementation for Selenium is different than mine here.

2. If your CR is declined and not merged then my MR is serving its original purpose. But everything else is true what you've mentioned here: it copies the Selenium code that might be changed/outdated, especially in ver. 4 (I haven't checked it yet); it might slow down the tests, etc. Maybe it would be better to proceed with your JS-implementation idea.

[asolntsev]

@bereg2k No problems in both cases. :)

1. If the PR gets merged (it will likely happen), it's still good to have your tests. We will just need to refactor them a bit.
2. Yes, these checks might slow down the tests, but it's better to have slow correct tests than fast incorrect. If this PR gets rejected, I would create another PR to extract some pieces of Select into smaller methods, so that we could reuse/call them in Selenide.

Yes, I will try to continue my work with JS implementation. We'll see how easy it is.

[bereg2k]

@asolntsev Ok, if you mean to make changes to this PR so it would work somehow, then yes, it's totally reasonable.

And if you need me to make any specific changes to this feature, let me know.

[asolntsev]

@bereg2k I have committed my JS implementation for review: #1876

[asolntsev]

@bereg2k The PR has been merged in Selenium. Apparently will be released soon.

[asolntsev]

@bereg2k FYI The check for disabled attribute of <select> and <option> was added in Selenium 4.5.0 (Selenide 6.9.0).
See commit ff88671fb8c and 25b30ffa665.

Though the implementation is a bit different from yours (it throws UnsupportedOperationException with not-so-readable error message. And it doesn't allow negative checks (because it throws exception right from constructor).

Let's leave this PR open because the discussion continues...

[bereg2k]

@asolntsev Thanks for the heads-up.
They went in the weird direction when removing the "assert" methods and placing that check into the Select constructor...

Would it be reasonable to catch such exceptions, and handle them a bit more gracefully to provide the ability for negative checks and readable assertion error messages?

[asolntsev]

@bereg2k Let's for wait the discussion result.
If it doesn't end up as we expect, we can always copy Select class and change it, or go on with JS implementation.

[asolntsev]

Overseeded by #1876