Skip to content

[python] replace misuses of typing.NoReturn#10462

Merged
AutomatedTester merged 3 commits intoSeleniumHQ:trunkfrom
symonk:fix-incorrect-no-return-types-in-chromium-options
Apr 8, 2022
Merged

[python] replace misuses of typing.NoReturn#10462
AutomatedTester merged 3 commits intoSeleniumHQ:trunkfrom
symonk:fix-incorrect-no-return-types-in-chromium-options

Conversation

@symonk
Copy link
Copy Markdown
Member

@symonk symonk commented Mar 20, 2022

Recent introduction of typing has a few incorrect use cases; This patch addresses misuses of typing.NoReturn which should be used when a function has no possibility of returning, mostly used incorrectly in place of None on various setters. I will audit the other uses of NoReturn throughout; consider this a start for now.

closes #10429 (eventually)

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.

@symonk symonk changed the title replace incorrect uses of typing.NoReturn in ChromiumOptions [python] replace incorrect uses of typing.NoReturn in ChromiumOptions Mar 20, 2022
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Mar 20, 2022

Codecov Report

Merging #10462 (a763e6e) into trunk (74f6344) will decrease coverage by 0.04%.
The diff coverage is 73.33%.

❗ Current head a763e6e differs from pull request most recent head 44ebe12. Consider uploading reports for the commit 44ebe12 to get more accurate results

@@            Coverage Diff             @@
##            trunk   #10462      +/-   ##
==========================================
- Coverage   45.05%   45.00%   -0.05%     
==========================================
  Files          85       85              
  Lines        5522     5519       -3     
  Branches      268      268              
==========================================
- Hits         2488     2484       -4     
- Misses       2766     2767       +1     
  Partials      268      268              
Impacted Files Coverage Δ
py/selenium/webdriver/firefox/webdriver.py 31.46% <33.33%> (-1.18%) ⬇️
py/selenium/webdriver/chromium/webdriver.py 53.48% <75.00%> (-0.54%) ⬇️
py/selenium/webdriver/chromium/options.py 65.27% <85.71%> (ø)
py/selenium/webdriver/ie/webdriver.py 64.78% <100.00%> (-0.49%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a76e185...44ebe12. Read the comment docs.


def quit(self) -> NoReturn:
def quit(self) -> None:
RemoteWebDriver.quit(self)
Copy link
Copy Markdown
Member Author

@symonk symonk Mar 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Todo: consider super() in this class in general, will address in a seperate ticket

@symonk symonk changed the title [python] replace incorrect uses of typing.NoReturn in ChromiumOptions [python] replace misuses of typing.NoReturn Mar 20, 2022
@AutomatedTester AutomatedTester merged commit 7217bb3 into SeleniumHQ:trunk Apr 8, 2022
@symonk symonk deleted the fix-incorrect-no-return-types-in-chromium-options branch April 8, 2022 12:40
elgatov pushed a commit to elgatov/selenium that referenced this pull request Jun 27, 2022
* replace incorrect uses of `typing.NoReturn` in `ChromiumOptions`

* fix other misuses of `typing.NoReturn`

Co-authored-by: David Burns <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: Python NoReturn type in Chromium Options

3 participants