Skip to content
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

Adding spaces -> FramesTest.java #2124

Merged
merged 2 commits into from
Jan 24, 2025
Merged

Conversation

AndreyJVM
Copy link
Contributor

@AndreyJVM AndreyJVM commented Jan 7, 2025

User description

Hi,

Description

Adding spaces

Motivation and Context

Visual perception of java code

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

PR Type

Bug fix


Description

  • Fixed spacing issues in variable declarations.

  • Improved code readability in FramesTest.java.


Changes walkthrough 📝

Relevant files
Bug fix
FramesTest.java
Fix spacing and formatting in FramesTest.java                       

examples/java/src/test/java/dev/selenium/interactions/FramesTest.java

  • Fixed spacing around the = operator in variable declarations.
  • Ensured consistent formatting for better readability.
  • Added a newline at the end of the file.
  • +3/-3     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    netlify bot commented Jan 7, 2025

    Deploy Preview for selenium-dev ready!

    Name Link
    🔨 Latest commit 060eaf4
    🔍 Latest deploy log https://app.netlify.com/sites/selenium-dev/deploys/67932a53c5902100089ced5d
    😎 Deploy Preview https://deploy-preview-2124--selenium-dev.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @CLAassistant
    Copy link

    CLAassistant commented Jan 7, 2025

    CLA assistant check
    All committers have signed the CLA.

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 7, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ No major issues detected

    Copy link
    Contributor

    qodo-merge-pro bot commented Jan 7, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Add explicit waits to improve test stability and reduce flakiness

    Add explicit waits before interacting with elements to ensure they are fully loaded
    and interactive, preventing potential flakiness.

    examples/java/src/test/java/dev/selenium/interactions/FramesTest.java [44-45]

    -WebElement emailE = driver.findElement(By.id("email"));
    +WebElement emailE = new WebDriverWait(driver, Duration.ofSeconds(10))
    +    .until(ExpectedConditions.elementToBeClickable(By.id("email")));
     emailE.sendKeys("[email protected]");
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding explicit waits is critical for UI test stability, as it properly handles dynamic page loading and prevents flaky tests. The suggestion provides a concrete implementation using WebDriverWait.

    9
    Add error handling for iframe switching operations to improve test robustness

    Add error handling around the iframe switch operation to handle cases where the
    iframe element is not found or not accessible. This will make the test more robust.

    examples/java/src/test/java/dev/selenium/interactions/FramesTest.java [39-41]

    -WebElement iframe = driver.findElement(By.id("iframe1"));
    -driver.switchTo().frame(iframe);
    +try {
    +    WebElement iframe = driver.findElement(By.id("iframe1"));
    +    driver.switchTo().frame(iframe);
    +} catch (NoSuchFrameException e) {
    +    throw new RuntimeException("Failed to switch to iframe", e);
    +}
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for iframe operations is crucial for test reliability, as iframe-related issues are common in web testing. The suggestion provides specific exception handling that would make debugging easier.

    8
    Improve assertion readability and error messaging by using more specific assertion methods

    Replace the hard-coded boolean comparison in assertEquals with direct assertion to
    improve test readability and error messages.

    examples/java/src/test/java/dev/selenium/interactions/FramesTest.java [42]

    -assertEquals(true, driver.getPageSource().contains("We Leave From Here"));
    +assertTrue(driver.getPageSource().contains("We Leave From Here"), "Expected text 'We Leave From Here' not found in page source");
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using assertTrue with a descriptive message instead of assertEquals(true, ...) provides clearer error messages and follows testing best practices, making test failures more informative and easier to debug.

    7

    @VietND96 VietND96 merged commit 3338405 into SeleniumHQ:trunk Jan 24, 2025
    12 checks passed
    selenium-ci added a commit that referenced this pull request Jan 24, 2025
    [deploy site]
    
    Co-authored-by: Viet Nguyen Duc <[email protected]> 3338405
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants