You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This fixes an issue where FedCM tests were failing in the internal Python test suite located in py/test/selenium/webdriver/common/fedcm_tests.py.
The tests would open the FedCM dialog, but never close it... which would lead to subsequent tests failing in odd ways. This PR adds code to dismiss the dialog in several tests after the test completes. Now state is not leaked and all tests pass.
🔄 Types of changes
Bug fix (backwards compatible)
PR Type
Bug fix, Tests
Description
Fixes FedCM tests leaking state by dismissing dialogs.
Adds dialog.dismiss() to multiple test cases to ensure cleanup.
Resolves test failures caused by unclosed FedCM dialogs.
Changes walkthrough 📝
Relevant files
Tests
fedcm_tests.py
Add dialog dismissal to FedCM tests
py/test/selenium/webdriver/common/fedcm_tests.py
Added dialog.dismiss() to ensure dialogs are closed after tests.
Updated multiple test cases to prevent state leakage.
Improved test reliability by cleaning up dialog state.
The test_dialog_cancel method is missing the actual dialog cancellation action and assertion to verify the cancellation behavior. Also, it doesn't dismiss the dialog, which could lead to state leakage.
def test_dialog_cancel(self, driver):
driver.execute_script("triggerFedCm();")
dialog = driver.fedcm_dialog()
+ dialog.cancel()+ # Add appropriate assertion here to verify cancellation+ # For example: assert some_condition
Apply this suggestion
Suggestion importance[1-10]: 8
__
Why: This suggestion identifies an incomplete test method that's missing both the actual cancellation action and verification. The improvement adds the missing dialog.cancel() call and suggests adding appropriate assertions, which would significantly improve test coverage and prevent potential state leakage.
Medium
General
Remove commented code
The commented out dialog.click_continue() line suggests an incomplete test implementation. Either remove the commented code if it's not needed or implement the continue functionality if it's part of the test's intent.
def test_select_account(self, driver):
driver.execute_script("triggerFedCm();")
dialog = driver.fedcm_dialog()
dialog.select_account(1)
driver.fedcm_dialog() # Wait for dialog to become interactable
- # dialog.click_continue()
dialog.dismiss()
Apply this suggestion
Suggestion importance[1-10]: 4
__
Why: The suggestion correctly identifies and removes commented code that appears to be unused. This improves code cleanliness and readability by eliminating potentially confusing commented code, though it has minimal impact on functionality.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
🔗 Related Issues
Fixes #15581
💥 What does this PR do?
This fixes an issue where FedCM tests were failing in the internal Python test suite located in
py/test/selenium/webdriver/common/fedcm_tests.py.The tests would open the FedCM dialog, but never close it... which would lead to subsequent tests failing in odd ways. This PR adds code to dismiss the dialog in several tests after the test completes. Now state is not leaked and all tests pass.
🔄 Types of changes
PR Type
Bug fix, Tests
Description
Fixes FedCM tests leaking state by dismissing dialogs.
Adds
dialog.dismiss()to multiple test cases to ensure cleanup.Resolves test failures caused by unclosed FedCM dialogs.
Changes walkthrough 📝
fedcm_tests.py
Add dialog dismissal to FedCM testspy/test/selenium/webdriver/common/fedcm_tests.py
dialog.dismiss()to ensure dialogs are closed after tests.