Skip to content

Conversation

navin772
Copy link
Member

@navin772 navin772 commented Apr 25, 2025

User description

🔗 Related Issues

💥 What does this PR do?

Enables working tests for edge browser for storage commands.
Enables tests for edge and chrome for permissions module.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Cleanup (formatting, renaming)

PR Type

Tests


Description

  • Enable BiDi storage tests for Edge browser

  • Remove Edge-specific test skips in storage tests

  • Add Edge to test suite browser matrix


Changes walkthrough 📝

Relevant files
Tests
StorageCommandsTest.java
Enable storage tests for Edge by removing skip annotations

java/test/org/openqa/selenium/bidi/storage/StorageCommandsTest.java

  • Removed @NotYetImplemented(EDGE) from multiple test methods
  • Enabled storage-related tests for Edge browser
  • No other test logic changes
  • +0/-6     
    Configuration changes
    BUILD.bazel
    Add Edge to test suite browser matrix                                       

    java/test/org/openqa/selenium/bidi/storage/BUILD.bazel

  • Added "edge" to the list of browsers for large test suite
  • Ensures storage tests run on Edge in CI
  • +1/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Apr 25, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented Apr 25, 2025

    PR Reviewer Guide 🔍

    (Review updated until commit 3e835bc)

    Here are some key observations to aid the review process:

    🎫 Ticket compliance analysis ❌

    5678 - Not compliant

    Non-compliant requirements:

    • Fix "Error: ConnectFailure (Connection refused)" when instantiating ChromeDriver multiple times

    Requires further human verification:

    • The PR is about enabling Edge tests for BiDi storage module, which seems unrelated to the ChromeDriver connection issue

    1234 - Not compliant

    Non-compliant requirements:

    • Fix issue where Selenium 2.48 doesn't trigger JavaScript in link's href on click()

    Requires further human verification:

    • The PR is about enabling Edge tests for BiDi storage module, which seems unrelated to the Firefox JavaScript click issue

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Configuration

    Verify that Edge browser is properly configured in CI environment to run these tests. Since these tests were previously skipped with @NotYetImplemented annotations, ensure the Edge WebDriver implementation now fully supports these BiDi storage operations.

        "edge",
    ],

    Copy link
    Contributor

    qodo-merge-pro bot commented Apr 25, 2025

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @navin772 navin772 marked this pull request as draft April 25, 2025 07:18
    @navin772
    Copy link
    Member Author

    The PermissionsTest is failing for chrome and edge in the CI but passes locally. I will be removing the permission module related changes in this PR and investigate it separately.

    @navin772 navin772 changed the title [java][bidi]: enable tests for storage and permissions module [java][bidi]: enable tests for storage module for edge Apr 28, 2025
    @navin772 navin772 marked this pull request as ready for review April 28, 2025 12:53
    @navin772 navin772 requested a review from pujagani April 28, 2025 12:53
    Copy link
    Contributor

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    Copy link
    Contributor

    @pujagani pujagani left a comment

    Choose a reason for hiding this comment

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

    Thank you @navin772

    @pujagani pujagani merged commit 94e0560 into SeleniumHQ:trunk May 5, 2025
    11 checks passed
    @navin772 navin772 deleted the java-bidi-enable-tests branch May 5, 2025 05:09
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related C-java Java Bindings Review effort 1/5
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants