adding the EpectedCondition invisibility_of_element

[larkost]

• By placing an X in the preceding checkbox, I verify that I have signed the Contributor License Agreement

This adds a invisibility_of_element to compliment the existing invisibility_of_element_located ExpectedCondition. It also makes both methods use the same logic where either a locator or a WebElement can be given to them to evaluate.

If people are happy with this approach I am happy to follow up giving other similar pairs the same treatment. I also did not see the documentation in this repository, so don't know where to add that. Pointers in that direction would be appreciated.

[lmtierney]

I personally am not in favor of adding any additional Expected Conditions, but I will leave this open for a while to gauge interest.

[larkost-fetch]

Right now the list of Expected Conditions looks a bit scatter-shot to me. But I was a bit annoyed that there was a visibility_of/visibility_of_element_located pair, but only invisibility_of_element_located, without the element-based method to pair there. I probably should have gone with the name invisibility_of, and would be happy to correct that.

Long term I would personally be in favor of cutting it down to just visibility_of/invisibility_of, where both take either a locator pair or an existing element reference (so what I am adding here). But removing the _element_located versions feels like something that should be done after a depreciation period, and probably at a major update.

[shs96c]

I think ExpectedConditions is already a little large. We can start slimming it down now, since we now extend the normal java.util.function.Function so people can use the standard Predicate.negate method to invert boolean conditions, and we already have ExpectedConditions.not. Closing because even though I like the symmetry, I think we should be removing stuff from this class rather than adding to it.

[larkost-fetch]

@shs96c This is Python, there is no Predicate.negate.

And if you really want less methods, then my approach of being generous with input seem to provide a clear method forward to consolidating a few of the Expected Conditions, without losing any functionality.

And seriously, are you going to remove the visibility/invisibility parts? I doubt it, so this is not additional clutter.

[larkost]

@shs96c, @innokenty I have to say that this was a pretty bad interaction with a first time commiter. I hope that this is not representative of the general culture in the project. Since I have some experience in well-run projects I will give you some notes:

1. While it is a bit touchy-feely, no one thanked me for taking the time to commit. While personally this is not really important to me, it is a generally considered good form, and I have seen how this does help grow a community.

2. The focus from the get-go appears to be how to get rid of the contribution. @shs96c's closing comments were about Java, and not applicable to Python. That reeks of a rush to close rather than good stewardship.

3. I offered a way forward (i.e.: consolidate methods), and rather than jump on that it was ignored. A well run project would try to funnel new energy into desired code, not cut it off at the knees.

4. There does not seem to have been any effort to answer my responses or comments. That is just rude. It has been 2 days, and no-one responded.

My employer is paying me to improve our use of Selenium, so I have a whole framework of updates that could be pushed back upstream. There seems little point to do so now.

[shs96c]

@larkost This is totally my bad. First of all, an apology: I didn't pay enough attention when I closed this PR --- for some reason I thought this was java --- and there's no excuse for that. Sorry.

You're also totally justified to want thanking. It wasn't polite not to thank you, especially since we normally make an effort to show some appreciation. Again, I apologise.

I'm glad that your employer is paying you to support Selenium. That's a fortunate and happy place to be in. I'm happy for you.

However, of the 6 people who have made more than 10 commits this year, none of us are paid to work on Selenium. We fit our Open Source work in around our work and personal commitments. There are one or two people who can review Python patches properly.

The fact that it took two days isn't rude: it's a sign that people are stressed and over-committed. I'm not singling you out here, since there are many, many people who believe the same, but it's a crying shame that people mistake a labour of love by deeply committed individuals for a corporate effort backed by plenty of dollars and hordes of engineers. When we get the core committers together, we can sit round a single table, and buying a round of drinks for them isn't expensive. We're exhausted.

[larkost]

@shs96c Thanks for re-looking at this. I completely understand the realities of open-source development. The only reason I focused on the 2 days thing is because the rest of the conversation was so fast. That difference caused it to look like "case closed". I am not in any hurry to get this approved (locally I already have workarounds), and to be clear I am happy to re-work it if should go in another direction.

And as I said... I am not a touchy-feely person in general. I care more that a process be based on good arguments. But I did witness first-hand that just a little bit of the soft-touch approach goes a long way, especially with new contributors. It felt that might be a lesson unlearned in this community. I am happy to hear that that is not the case.

I do notice that this was not un-closed. Do you want me to re-submit?

[davehunt]

Something that's been on my to-do list for a long time is to write a Python package that provides a library of expected conditions for use with Selenium. I'm not a fan of the inconsistencies in the support package, and would prefer that we had a decent third-party package to recommend.

That said, there's no reason why a rewrite couldn't exist in the Selenium project if we had someone with the time to contribute to and maintain it. We could even deprecate the existing package, and just have it call through to the new code.

[larkost]

@davehunt Could you list out the conditions that you would like to see in such a package? My own needs have been mostly centred around visibility/invisibility, but I might take up that challenge if I could get a good idea of what it looked like.

[davehunt]

@larkost I'd start with the conditions that are provided by the existing support module, but taking a more Pythonic approach (I honestly haven't thought about this too much, so don't know what it would look like). I would focus on a consistent approach to naming, arguments, and return values. Good test coverage, and thorough documentation with plenty of examples.

After taking a look at this patch, I'd b personally be okay with landing it as it's consistent with the existing conditions. I'll reopen, and add my review.

[davehunt]

This is causing the tests to fail with AttributeError: 'invisibility_of_element' object has no attribute 'locator'

[larkost]

I made fixes, but at least some of the CI failures are not related. I am having trouble getting the tests to run locally, and am not sure if it is my setup, or something I am missing.

[lmtierney]

Failures are not related to your change. You should be able to run tests if you follow the steps here: https://github.com/SeleniumHQ/selenium/wiki/Python-Bindings#development

You can also run them with pytest outside of tox if you supply the --driver=Chrome, etc arg

[lmtierney]

ping @davehunt it looks like your review has been addressed

[davehunt]

Merged as 7842db3