Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

added frame ruby example #2337

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
rpallavisharma merged 9 commits into SeleniumHQ:trunk from rpallavisharma:fixExample-pal
Jun 7, 2025

Conversation

@rpallavisharma
Copy link
Member

@rpallavisharma rpallavisharma commented Jun 7, 2025 •
edited
Loading

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.

Description

added frame ruby example

Motivation and Context

added frame ruby example

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

Tests

Description

  • Added comprehensive Ruby example for iframe interactions

  • Introduced RSpec test for frame switching and element access

  • Validated iframe access by id, name, and index

  • Ensured page content checks and driver cleanup

Changes walkthrough 📝

Relevant files
Tests
frames_spec.rb
Add RSpec test for iframe/frame switching in Ruby

examples/ruby/spec/interactions/frames_spec.rb

  • Added new RSpec test for iframe switching in Ruby
  • Demonstrated switching by WebElement, name, and index
  • Included element interaction and content assertions
  • Ensured driver cleanup after test
    • +53/-0
    Formatting
    cookies_spec.rb
    Add blank lines for formatting consistency

    examples/ruby/spec/interactions/cookies_spec.rb

    • Added two blank lines at the file start for formatting
    +2/-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.
    •

    Copy link

    netlify bot commented Jun 7, 2025 •
    edited
    Loading

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 9e3c8e0

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 7, 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

    Redundant Driver

    The test creates a new Firefox driver instance despite having a driver already defined in the let block. This could lead to resource leaks if both are used.

     
    driver = Selenium::WebDriver.for :firefox
driver.get("https://www.selenium.dev/selenium/web/iframes.html")
    Inconsistent Usage

    The code locates an iframe by name (iframe1-name) but then switches to the previously defined iframe variable instead of using the newly located element.

     
    iframe1 = driver.find_element(:name, "iframe1-name") # (This line doesn't switch, just locates)
driver.switch_to.frame(iframe)
    Missing Assertion

    After sending keys to email elements, there's no verification that the input was successful before clearing it, reducing test effectiveness.

     
    email_element.send_keys("admin@selenium.dev")
email_element.clear

    Copy link
    Contributor

    qodo-merge-pro bot commented Jun 7, 2025 •
    edited
    Loading

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion Impact
    Possible issue
    ✅ (削除) Fix incorrect variable usage (削除ここまで)
    Suggestion Impact:The commit implemented the suggested fix by changing 'driver.switch_to.frame(iframe)' to 'driver.switch_to.frame(iframe1)' on line 28, ensuring the test correctly validates switching frames by name

    code diff:

    - iframe1 = driver.find_element(:name, "iframe1-name") # (This line doesn't switch, just locates)
- driver.switch_to.frame(iframe)
- expect(driver.page_source).to include("We Leave From Here")
+ iframe1 = driver.find_element(:name, 'iframe1-name') # (This line doesn't switch, just locates)
+ driver.switch_to.frame(iframe1)
+ expect(driver.page_source).to include('We Leave From Here')

    The code is using the previously defined iframe variable instead of the newly
    located iframe1. This will not test switching by name as intended. Use the
    iframe1 variable in the frame switch operation.

    examples/ruby/spec/interactions/frames_spec.rb [39-41] 

     # --- Switch to iframe using name or ID ---
 iframe1 = driver.find_element(:name, "iframe1-name") # (This line doesn't switch, just locates)
-driver.switch_to.frame(iframe)
+driver.switch_to.frame(iframe1)

    [Suggestion processed]

    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly identifies a logical bug where iframe is used instead of iframe1, causing the test to not properly validate switching by name. This is a valid bug fix that ensures the test works as intended.

    		Medium
    ✅ (削除) Prevent driver resource leak (削除ここまで)
    Suggestion Impact:The commit commented out the line that created a new Firefox driver instance, effectively using the driver from the let statement as suggested

    code diff:

    - driver = Selenium::WebDriver.for :firefox
- driver.get("https://www.selenium.dev/selenium/web/iframes.html")
+ #driver = Selenium::WebDriver.for :firefox
+ driver.get('https://www.selenium.dev/selenium/web/iframes.html')

    The test creates a new Firefox driver instance that overrides the one from the
    let(:driver) statement. This can cause resource leaks and inconsistent test
    behavior. Use the driver from the let statement instead.

    examples/ruby/spec/interactions/frames_spec.rb [25-27] 

     it 'performs iframe switching operations' do
- # Set firefox and launch web page
- driver = Selenium::WebDriver.for :firefox
+ # Launch web page using the driver from let statement
+ driver.get("https://www.selenium.dev/selenium/web/iframes.html")

    [Suggestion processed]

    Suggestion importance[1-10]: 5

    __

    Why: The suggestion identifies a resource management issue where a new driver instance overrides the let(:driver). However, the test appears to intentionally manage its own Firefox driver and calls driver.quit at the end, making this more of a style improvement than a critical issue.

    		Low
    • Update

    Copy link
    Member Author

    the lint error is in unrelated file of cookie_spec. cannot change it as it will need then change in all document files for cookie. raise another pr for this.

    Copy link
    Member Author

    the windows nightly error is also in unrelated file - spec/browsers/internet_explorer_spec.rb

    @rpallavisharma rpallavisharma merged commit 2795b67 into SeleniumHQ:trunk Jun 7, 2025
    8 of 10 checks passed
    Copy link
    Member

    diemol commented Jun 7, 2025

    Please don't merge if any job is failing.
    Also avoid code comments because they are en English and the whole point of having translations is lost.

    Copy link
    Member Author

    @diemol i wrote reason for the failing jobs. Kindly read them and let me know .
    The reason were unrelated to this that's why I merged.

    Please help me understand if it's incorrect?

    I will remove comments from code.

    I didn't know.

    Copy link
    Member

    diemol commented Jun 8, 2025

    The linter failed because this PR added two blank lines at the beginning of the cookies spec file.

    Copy link
    Member

    diemol commented Jun 8, 2025

    Additionally, the PR broke the build because the referenced file in the markdown was frames_spec.cs instead of frames_spec.rb.

    This was not detected in the PR checks because when a PR comes from a fork, it is not possible to use the GITHUB token to render the code examples. Please also run the hugo server command locally to double check the site builds, or create the PR from a branch in this repository (you are a committer and have the rights to do that).

    Copy link
    Member Author

    First of all I am sorry to break the build.
    I did run Hugo server.
    I ran all checks
    Saw all errors before I merged
    I understand that the file name had incorrect extension and no checks picked it.
    I understand I am a committer and I also in my best capacity assure you Diego that I followed all steps of checks but still a mistake happened
    I apologise for this @diemol.

    I will fix the file extension problem and remove the comments

    Beside this if there is any other problem you have found which went pass the checks and I am not aware please kindly let me know.

    Thank you for your kind review and time for this .

    Copy link
    Member

    diemol commented Jun 8, 2025

    If you run Hugo server please make sure that you have the token as an environment variable to the code examples are rendered.

    The easiest would be to create the PR from a branch in this repository and not your fork.

    Copy link
    Member

    diemol commented Jun 8, 2025

    Oh, and I did fix the lint error and the file name error already. No problem.

    Copy link
    Member Author

    Ok Diego @diemol
    Thank you for the fix.
    I understood what you have mentioned.

    @rpallavisharma rpallavisharma deleted the fixExample-pal branch June 9, 2025 06:25
    @rpallavisharma rpallavisharma restored the fixExample-pal branch June 9, 2025 06:25
    @rpallavisharma rpallavisharma deleted the fixExample-pal branch June 9, 2025 06:34
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    No reviews

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    AltStyle によって変換されたページ (->オリジナル) /