PEP484 type hints for py/selenium/webdriver/chrome/options.py

[thecr8tr]

Description

This PR adds type hints to selenium.webdriver.chrome.options

Motivation and Context

This is an action towards #9480.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• I have read the contributing document.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All new and existing tests passed.

[CLAassistant]

All committers have signed the CLA.

[titusfortner]

Should this be None instead of NoReturn?

[titusfortner]

@colons you have a very different implementation for enable_mobile for Firefox. (I'm assuming Chrome vs Firefox shouldn't be different)...

selenium/py/selenium/webdriver/firefox/options.py

Line 131 in fb2b991

self, android_package: Optional[str] = "org.mozilla.firefox", android_activity: str = None, device_serial: str = None

[colons]

@titusfortner im not sure what difference exactly you're worried about here; the wrapping of optional kwargs with Optional is a matter of preference, for instance. annotating BaseOptions.enable_mobile() would be a good idea if you want mypy to complain at you if your subclasses are doing incompatible things

certainly, though, NoReturn is incorrect here, and i hope #10202 will make it easier to notice when new annotations are wrong

[titusfortner]

@colons — I thought everything in Python had a one true way for things. :)
Thanks for clarifying and providing the link.

[titusfortner]

This needs to be None not NoReturn

[thecr8tr]

I corrected that and pushed it. Should we be revisiting NoReturn usage in other places such as https://github.com/SeleniumHQ/selenium/blob/trunk/py/selenium/webdriver/firefox/webdriver.py#L186 ?

[titusfortner]

Yes! @colons already did that here — #10202
I was trying to figure out why the bazel build //py:test-remote is failing before merging that one, but as discussed on Slack, I can't seem to get it to run on my machine at all.