Skip to content

Add test case for Select visibility issue #15265 #15694

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

Merrn60
Copy link

@Merrn60 Merrn60 commented May 3, 2025

User description

Title: Add test case for Select visibility issue #15265

Description

This PR addresses the inconsistency in the Select class behavior across bindings, as reported in Issue #15265. It introduces a new Java test case (SelectHiddenOptionsTest.java) to ensure selectByVisibleText() skips hidden options (defined by CSS properties like display: none, visibility: hidden, opacity: 0).

Changes

  • Added SelectHiddenOptionsTest.java with a custom isVisible method to validate CSS visibility properties.
  • Included a sample HTML file (test_select.html) to simulate hidden options for testing.
  • The test checks for display, visibility, and opacity properties to determine element visibility.

Testing

  • Ran the test locally using ChromeDriver with the provided HTML file.
  • Verified that selectByVisibleText() fails to select hidden options, aligning with the expected behavior.
  • Used Maven (mvn test) to ensure compatibility with the project build.

Checklist

Notes

  • This is my first contribution to Selenium, so I'm open to feedback!
  • The test can be extended to cover selectByContainsVisibleText() if needed.
  • Please review the isVisible method for edge cases (e.g., inherited CSS or partial opacity).

Related Issue

Closes #15265


PR Type

Tests


Description

  • Add Java test for Select skipping hidden options

  • Include HTML fixture with visible and hidden options

  • Validate Select behavior for display, visibility, opacity


Changes walkthrough 📝

Relevant files
Tests
SelectHiddenOptionsTest.java
Add Java test for Select skipping hidden options                 

java/test/org/openqa/selenium/testing/SelectHiddenOptionsTest.java

  • Introduces a Java test class to verify Select skips hidden options
  • Implements custom isVisible method for CSS checks
  • Tests selectByVisibleText with various hidden/visible options
  • Uses ChromeDriver and local HTML file for testing
  • +86/-0   
    test_select.html
    Add HTML fixture for Select hidden options test                   

    java/test/org/openqa/selenium/testing/test_select.html

  • Adds HTML file with select element and options
  • Defines options hidden by display, visibility, and opacity
  • Used as fixture for SelectHiddenOptionsTest
  • +18/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • @CLAassistant
    Copy link

    CLAassistant commented May 3, 2025

    CLA assistant check
    All committers have signed the CLA.

    @selenium-ci selenium-ci added the C-java Java Bindings label May 3, 2025
    Copy link
    Contributor

    qodo-merge-pro bot commented May 3, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

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

    Hardcoded Path

    The test uses a hardcoded file path to the HTML test file which will fail on other systems. Should use a relative path or resource loading mechanism instead.

    driver.get("file:///C:/Users/dell/OneDrive/Desktop/test_select.html");
    WebElement selectElement = driver.findElement(By.id("mySelect"));
    Test Framework

    The test is implemented as a main method rather than using a proper test framework like JUnit or TestNG which is standard for Selenium's test suite.

    public static void main(String[] args) {
        WebDriverManager.chromedriver().setup();
        ChromeOptions options = new ChromeOptions();
        options.addArguments("--remote-allow-origins=*");
        WebDriver driver = new ChromeDriver(options);
        try {
            driver.get("file:///C:/Users/dell/OneDrive/Desktop/test_select.html");
            WebElement selectElement = driver.findElement(By.id("mySelect"));
            Select select = new Select(selectElement);
    
            // Run test cases
            testOption(select, "Visible Option", true);
            testOption(select, "Hidden Option (display:none)", false);
            testOption(select, "Hidden Option (visibility:hidden)", false);
            testOption(select, "Hidden Option (opacity:0)", false);
    
            System.out.println("✅ Test execution completed. Press Enter to close the browser...");
            new Scanner(System.in).nextLine();
        } finally {
            driver.quit();
        }
    }
    Package Name

    The package name is 'org.example' but the file is located in 'org.openqa.selenium.testing', which creates a mismatch between package declaration and directory structure.

    package org.example;

    Copy link
    Contributor

    qodo-merge-pro bot commented May 3, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    General
    Remove hardcoded file path

    Hardcoded file paths make tests non-portable. Use a relative path or resource
    loading mechanism to access the test HTML file that's included in the
    repository.

    java/test/org/openqa/selenium/testing/SelectHiddenOptionsTest.java [21]

    -driver.get("file:///C:/Users/dell/OneDrive/Desktop/test_select.html");
    +// Use a relative path or resource loading
    +String testHtmlPath = SelectHiddenOptionsTest.class.getResource("/test_select.html").getFile();
    +driver.get("file://" + testHtmlPath);
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly points out that the hardcoded absolute file path makes the test non-portable and brittle. Using resource loading (getResource) as suggested is crucial for ensuring the test can run reliably across different environments and in CI/CD pipelines.

    High
    Remove manual user interaction

    Automated tests shouldn't require manual intervention. Remove the Scanner that
    waits for user input, as this prevents the test from running automatically in
    CI/CD pipelines.

    java/test/org/openqa/selenium/testing/SelectHiddenOptionsTest.java [31-32]

    -System.out.println("✅ Test execution completed. Press Enter to close the browser...");
    -new Scanner(System.in).nextLine();
    +System.out.println("✅ Test execution completed.");
    +// Remove the Scanner to allow automated test execution
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    __

    Why: The suggestion correctly identifies that requiring user input via Scanner prevents the test from being fully automated. Removing this manual step is essential for integrating the test into automated test suites and CI/CD pipelines.

    High
    Fix package declaration

    The package name should match the file path. Since the file is in
    java/test/org/openqa/selenium/testing/, the package should be
    org.openqa.selenium.testing instead of org.example.

    java/test/org/openqa/selenium/testing/SelectHiddenOptionsTest.java [1-3]

    -package org.example;
    +package org.openqa.selenium.testing;
     
     import io.github.bonigarcia.wdm.WebDriverManager;
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    __

    Why: The suggestion correctly identifies that the package declaration org.example does not match the file's directory structure (org/openqa/selenium/testing). Adhering to this convention is important for code organization and preventing potential classpath issues.

    Low
    Learned
    best practice
    Add parameter null checks

    Add null checks for the select and text parameters at the beginning of the
    method to prevent NullReferenceExceptions. This ensures the method fails
    gracefully with clear error messages when null parameters are provided.

    java/test/org/openqa/selenium/testing/SelectHiddenOptionsTest.java [66-74]

     private static WebElement findOptionByText(Select select, String text) {
    +    if (select == null) {
    +        throw new IllegalArgumentException("Select object cannot be null");
    +    }
    +    if (text == null) {
    +        throw new IllegalArgumentException("Text parameter cannot be null");
    +    }
    +    
         List<WebElement> options = select.getOptions();
         for (WebElement option : options) {
             if (text.equals(option.getText())) {
                 return option;
             }
         }
         return null;
     }
    • Apply this suggestion
    Suggestion importance[1-10]: 6
    Low
    • Update

    @Merrn60
    Copy link
    Author

    Merrn60 commented May 3, 2025

    @shbenzer

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: Unifying Select Class Across All Bindings
    3 participants