Skip to content

[py] added type hinting support to ActionChains and Alert class#12290

Merged
AutomatedTester merged 6 commits intoSeleniumHQ:trunkfrom
sandeepsuryaprasad:type-hints
Jul 12, 2023
Merged

[py] added type hinting support to ActionChains and Alert class#12290
AutomatedTester merged 6 commits intoSeleniumHQ:trunkfrom
sandeepsuryaprasad:type-hints

Conversation

@sandeepsuryaprasad
Copy link
Copy Markdown
Contributor

@sandeepsuryaprasad sandeepsuryaprasad commented Jun 29, 2023

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Added Type Hints to all methods in ActionChains and Alert class

Motivation and Context

Added Type Hints to all methods in ActionChains and Alert class

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.

mykola-mokhnach
mykola-mokhnach reviewed Jun 30, 2023
View reviewed changes
Comment thread py/selenium/webdriver/common/action_chains.py Outdated
mykola-mokhnach
mykola-mokhnach reviewed Jun 30, 2023
View reviewed changes
Comment thread py/selenium/webdriver/common/action_chains.py Outdated
mykola-mokhnach
mykola-mokhnach reviewed Jun 30, 2023
View reviewed changes
Comment thread py/selenium/webdriver/common/action_chains.py Outdated
mykola-mokhnach
mykola-mokhnach reviewed Jun 30, 2023
View reviewed changes
Comment thread py/selenium/webdriver/common/action_chains.py
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jul 1, 2023 •
edited
Loading

Codecov Report

Patch coverage: 79.54% and project coverage change: +0.92 🎉

Comparison is base (47f993f) 56.92% compared to head (d58f9b7) 57.84%.

❗ Current head d58f9b7 differs from pull request most recent head 5aa4b01. Consider uploading reports for the commit 5aa4b01 to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files 
@@            Coverage Diff             @@
##            trunk   #12290      +/-   ##
==========================================
+ Coverage   56.92%   57.84%   +0.92%     
==========================================
  Files          86       86              
  Lines        5423     5437      +14     
  Branches      193      193              
==========================================
+ Hits         3087     3145      +58     
+ Misses       2143     2099      -44     
  Partials      193      193
Impacted Files Coverage Δ
py/selenium/webdriver/common/action_chains.py 62.25% <78.94%> (+32.32%) ⬆️
py/selenium/webdriver/common/alert.py 53.84% <80.00%> (+30.76%) ⬆️
py/selenium/webdriver/ie/options.py 47.50% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

AutomatedTester
AutomatedTester requested changes Jul 4, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@AutomatedTester AutomatedTester left a comment

Choose a reason for hiding this comment

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

the missing types that @mykola-mokhnach pointed out still need doing before we can merge

@sandeepsuryaprasad
Copy link
Copy Markdown
Contributor Author

sandeepsuryaprasad commented Jul 4, 2023

the missing types that @mykola-mokhnach pointed out still need doing before we can merge

I have incorporated all the comments given by @mykola-mokhnach

@AutomatedTester
Copy link
Copy Markdown
Member

AutomatedTester commented Jul 4, 2023

The actions tests are now failing. You can run them with //py:test-chrome-test/selenium/webdriver/common/interactions_tests.py

@sandeepsuryaprasad
Copy link
Copy Markdown
Contributor Author

sandeepsuryaprasad commented Jul 4, 2023 •
edited
Loading

The actions tests are now failing. You can run them with //py:test-chrome-test/selenium/webdriver/common/interactions_tests.py

I fixed it. I forgot to initialise the functions that was taking Optional WebElement to None. Now it's fixed. All tests are passing in my local.

@AutomatedTester AutomatedTester merged commit 2764ea7 into SeleniumHQ:trunk Jul 12, 2023
@sandeepsuryaprasad sandeepsuryaprasad deleted the type-hints branch September 17, 2023 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AutomatedTester AutomatedTester AutomatedTester approved these changes

+2 more reviewers

@mykola-mokhnach mykola-mokhnach mykola-mokhnach left review comments

@MarcelWilson MarcelWilson MarcelWilson left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@sandeepsuryaprasad @codecov-commenter @AutomatedTester @mykola-mokhnach @MarcelWilson