Skip to content

added code and content for frames #1915

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

Merged
merged 4 commits into from
Sep 2, 2024

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Aug 30, 2024

User description

Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, and help reviewers by making them as simple and short as possible.

added content and code for frames example

Description

added content and code for frames example

Motivation and Context

added content and code for frames example as it wasn't there

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

enhancement, documentation


Description

  • Added a new Java test class FramesTest to demonstrate iframe interactions using Selenium.
  • Implemented a comprehensive test method to switch between iframes using different methods: WebElement, name, and index.
  • Updated documentation in multiple languages (English, Japanese, Portuguese, Chinese) to reference the new Java test file for iframe examples.
  • Replaced inline Java code in documentation with references to the new test file, ensuring consistency and clarity.

Changes walkthrough 📝

Relevant files
Enhancement
FramesTest.java
Add Java test for iframe interactions using Selenium         

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

  • Added a new test class FramesTest for handling iframes.
  • Implemented a test method informationWithElements to demonstrate
    switching between iframes.
  • Utilized different methods for switching to iframes: by WebElement,
    name, and index.
  • Included assertions to verify iframe content and interactions.
  • +70/-3   
    Documentation
    frames.en.md
    Update English documentation with iframe example references

    website_and_docs/content/documentation/webdriver/interactions/frames.en.md

  • Updated documentation to include example code for switching iframes.
  • Replaced inline Java code with references to the new Java test file.
  • +15/-27 
    frames.ja.md
    Update Japanese documentation with iframe example references

    website_and_docs/content/documentation/webdriver/interactions/frames.ja.md

  • Updated Japanese documentation to include example code for switching
    iframes.
  • Replaced inline Java code with references to the new Java test file.
  • +9/-25   
    frames.pt-br.md
    Update Portuguese documentation with iframe example references

    website_and_docs/content/documentation/webdriver/interactions/frames.pt-br.md

  • Updated Portuguese documentation to include example code for switching
    iframes.
  • Replaced inline Java code with references to the new Java test file.
  • +9/-25   
    frames.zh-cn.md
    Update Chinese documentation with iframe example references

    website_and_docs/content/documentation/webdriver/interactions/frames.zh-cn.md

  • Updated Chinese documentation to include example code for switching
    iframes.
  • Replaced inline Java code with references to the new Java test file.
  • +12/-26 

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Sorry, something went wrong.

    Verified

    This commit was created on GitHub.com and signed with GitHub’s verified signature.
    Copy link

    netlify bot commented Aug 30, 2024

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit ab5111c

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Aug 30, 2024
    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Resource Management
    The WebDriver instance is not properly closed using try-with-resources or a finally block, which may lead to resource leaks.

    Test Structure
    The test method contains multiple assertions and operations, which violates the single responsibility principle for unit tests. Consider splitting into multiple test methods.

    Hardcoded Wait
    The use of implicitlyWait with a fixed duration may lead to unreliable tests. Consider using explicit waits for better test stability.

    Copy link
    Contributor

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Use try-with-resources to automatically close the WebDriver

    Use a try-with-resources statement to ensure the WebDriver is closed properly, even
    if an exception occurs.

    examples/java/src/test/java/dev/selenium/interactions/FramesTest.java [31-71]

    -WebDriver driver = new ChromeDriver();
    -driver.manage().timeouts().implicitlyWait(Duration.ofMillis(500));
    +try (WebDriver driver = new ChromeDriver()) {
    +    driver.manage().timeouts().implicitlyWait(Duration.ofMillis(500));
    +    
    +    // Navigate to Url
    +    driver.get("https://www.selenium.dev/selenium/web/iframes.html");
    +    
    +    ...
    +    
    +    // No need for explicit quit() call
    +}
     
    -// Navigate to Url
    -driver.get("https://www.selenium.dev/selenium/web/iframes.html");
    -
    -...
    -
    -//quit the browser
    -driver.quit();
    -
    Suggestion importance[1-10]: 9

    Why: The suggestion to use try-with-resources ensures that the WebDriver is closed properly, even if an exception occurs, which is a best practice for resource management in Java.

    9
    Replace implicit wait with explicit wait for better control and reliability

    Use a WebDriverWait instead of implicit wait to improve test reliability and
    performance.

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

    -driver.manage().timeouts().implicitlyWait(Duration.ofMillis(500));
    +WebDriverWait wait = new WebDriverWait(driver, Duration.ofSeconds(10));
    +wait.until(ExpectedConditions.presenceOfElementLocated(By.id("iframe1")));
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Using WebDriverWait instead of an implicit wait provides more precise control over wait conditions, improving test reliability and performance.

    8
    Enhancement
    Use a more specific assertion for checking text content

    Replace the boolean assertion with a more specific assertion that checks for the
    exact text content.

    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: The suggestion improves the clarity of the test by using assertTrue with a descriptive message, making it easier to understand test failures.

    7
    Combine redundant frame switching tests into a single, more efficient test method

    Combine the two separate frame switching tests into a single test method to reduce
    redundancy and improve test efficiency.

    examples/java/src/test/java/dev/selenium/interactions/FramesTest.java [38-59]

    -//switch To IFrame using Web Element
    +// Test frame switching using both WebElement and name/id
     WebElement iframe = driver.findElement(By.id("iframe1"));
    -//Switch to the frame
    +
    +// Switch to frame using WebElement
     driver.switchTo().frame(iframe);
    -assertEquals(true, driver.getPageSource().contains("We Leave From Here"));
    -//Now we can type text into email field
    -WebElement emailE= driver.findElement(By.id("email"));
    -emailE.sendKeys("[email protected]");
    -emailE.clear();
    +assertTrue(driver.getPageSource().contains("We Leave From Here"), "Expected text not found after switching by WebElement");
    +WebElement emailElement = driver.findElement(By.id("email"));
    +emailElement.sendKeys("[email protected]");
    +emailElement.clear();
     driver.switchTo().defaultContent();
     
    -
    -//switch To IFrame using name or id
    -driver.findElement(By.name("iframe1-name"));
    -//Switch to the frame
    -driver.switchTo().frame(iframe);
    -assertEquals(true, driver.getPageSource().contains("We Leave From Here"));
    -WebElement email=driver.findElement(By.id("email"));
    -//Now we can type text into email field
    -email.sendKeys("[email protected]");
    -email.clear();
    +// Switch to frame using name
    +driver.switchTo().frame("iframe1-name");
    +assertTrue(driver.getPageSource().contains("We Leave From Here"), "Expected text not found after switching by name");
    +emailElement = driver.findElement(By.id("email"));
    +emailElement.sendKeys("[email protected]");
    +emailElement.clear();
     driver.switchTo().defaultContent();
     
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion reduces redundancy and improves test efficiency by combining similar frame switching tests, although it may slightly reduce test granularity.

    6

    Sorry, something went wrong.

    Copy link
    Member

    @harsha509 harsha509 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 @pallavigitwork !

    @pallavigitwork
    Copy link
    Member Author

    Thank you @harsha509 :)

    harsha509
    harsha509 previously approved these changes Sep 2, 2024
    Copy link
    Member

    @harsha509 harsha509 left a comment

    Choose a reason for hiding this comment

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

    LGTM!

    Thanks @pallavigitwork !

    @harsha509 harsha509 dismissed their stale review September 2, 2024 13:18

    I see there are few tests failing looking into it!

    @harsha509 harsha509 merged commit 7477ac9 into SeleniumHQ:trunk Sep 2, 2024
    9 checks passed
    selenium-ci added a commit that referenced this pull request Sep 2, 2024
    Co-authored-by: Sri Harsha <[email protected]> 7477ac9
    @pallavigitwork pallavigitwork deleted the pallavi-framecode branch September 27, 2024 12:08
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 3
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    None yet

    2 participants