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
Enables test_add_event_handler_download_end and test_add_event_handler_download_will_begin tests for Firefox.
From Firefox 145, these events are supported.
Also, makes the assertion generalized in case of existing downloads.
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
Cleanup (formatting, renaming)
PR Type
Tests
Description
Remove xfail markers for Firefox download event tests
Generalize filename assertion to handle existing downloads
Support Firefox 145+ download event functionality
Diagram Walkthrough
flowchart LR
A["Firefox 145+ Support"] --> B["Remove xfail Markers"]
B --> C["test_add_event_handler_download_will_begin"]
B --> D["test_add_event_handler_download_end"]
E["Generalize Assertion"] --> F["Handle Filename Variations"]
F --> C
Loading
File Walkthrough
Relevant files
Tests
bidi_browsing_context_tests.py
Enable Firefox download event tests with generalized assertions
Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance.
Status: No audit logs: The new test code adds assertions and event handling for downloads without introducing or verifying any audit logging for critical actions, which may be acceptable for tests but cannot be confirmed from this diff.
Referred Code
assertlen(events_received) ==1# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dirassert"file_1"inevents_received[0].suggested_filenamedriver.browsing_context.remove_event_handler("download_will_begin", callback_id)
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation
Status: Limited edge handling: The generalized filename assertion only checks for "file_1" substring and relies on a 5s wait without explicit error handling or alternate outcomes, which may be acceptable in tests but lacks explicit edge-case handling in this diff.
Referred Code
WebDriverWait(driver, 5).until(lambdad: len(events_received) >0)
assertlen(events_received) ==1# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dirassert"file_1"inevents_received[0].suggested_filename
Description: The assertion on suggested_filename is loosened to a substring match ("file_1"), which could allow false positives and mask regressions in download handling; consider validating against a stricter expected pattern to avoid missing discrepancies. bidi_browsing_context_tests.py [803-806]
Referred Code
assertlen(events_received) ==1# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dirassert"file_1"inevents_received[0].suggested_filename
Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance.
Status: No audit logs: The new assertions and event handling in tests add no audit logging for critical actions, but as test code this may be acceptable and outside production audit requirements.
Referred Code
assertlen(events_received) ==1# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dirassert"file_1"inevents_received[0].suggested_filenamedriver.browsing_context.remove_event_handler("download_will_begin", callback_id)
Generic: Robust Error Handling and Edge Case Management
Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation
Status: Minimal error handling: The test relies on assertions and a short wait without explicit handling for timing or environment edge cases, which is typical for tests but may cause flakiness.
Referred Code
WebDriverWait(driver, 5).until(lambdad: len(events_received) >0)
assertlen(events_received) ==1# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dirassert"file_1"inevents_received[0].suggested_filename
download_xpath_file_1_txt = '//*[@id="file-1"]'
driver.find_element(By.XPATH, download_xpath_file_1_txt).click()
WebDriverWait(driver, 5).until(lambda d: len(events_received) > 0)
++# Allow a moment for any unexpected additional events to arrive.+time.sleep(0.2)
assert len(events_received) == 1
# filename maybe file_1.txt or file_1(1).txt depending on existing files in download dir
assert "file_1" in events_received[0].suggested_filename
[To ensure code accuracy, apply this suggestion manually]
Suggestion importance[1-10]: 6
__
Why: The suggestion correctly identifies a potential race condition in an asynchronous test and proposes adding a short delay to improve its stability, which is a valid approach to prevent test flakiness.
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
💥 What does this PR do?
Enables
test_add_event_handler_download_endandtest_add_event_handler_download_will_begintests for Firefox.From Firefox 145, these events are supported.
Also, makes the assertion generalized in case of existing downloads.
🔧 Implementation Notes
💡 Additional Considerations
🔄 Types of changes
PR Type
Tests
Description
Remove xfail markers for Firefox download event tests
Generalize filename assertion to handle existing downloads
Support Firefox 145+ download event functionality
Diagram Walkthrough
File Walkthrough
bidi_browsing_context_tests.py
Enable Firefox download event tests with generalized assertionspy/test/selenium/webdriver/common/bidi_browsing_context_tests.py
@pytest.mark.xfail_firefoxdecorator fromtest_add_event_handler_download_will_begintest@pytest.mark.xfail_firefoxdecorator fromtest_add_event_handler_download_endtestequality