Skip to content

Conversation

sandeepsuryaprasad
Copy link
Contributor

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.

@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2023

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.

Copy link
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
Contributor Author

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
Member

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

@sandeepsuryaprasad
Copy link
Contributor Author

sandeepsuryaprasad 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

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants