Skip to content

Cookie code for ruby #2230

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

Closed

Conversation

pallavigitwork
Copy link
Member

@pallavigitwork pallavigitwork commented Mar 21, 2025

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

Cookie code for ruby

Motivation and Context

Cookie code for ruby

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, Tests, Documentation


Description

  • Added comprehensive Ruby tests for cookies and frames interactions.

    • Includes adding, retrieving, deleting, and managing cookies.
    • Covers iframe interactions using various methods (ID, name, index).
  • Updated documentation to reference new Ruby examples for cookies and frames.

    • Replaced inline Ruby code with links to example files.
    • Updated multiple language versions of the documentation.
  • Adjusted Hugo configuration to ignore specific errors.


Changes walkthrough 📝

Relevant files
Tests
2 files
cookies_spec.rb
Add Ruby tests for cookies management                                       
+77/-1   
frames_spec.rb
Add Ruby tests for iframe interactions                                     
+40/-2   
Documentation
8 files
cookies.en.md
Update English cookies documentation with Ruby examples   
+24/-81 
cookies.ja.md
Update Japanese cookies documentation with Ruby examples 
+20/-81 
cookies.pt-br.md
Update Portuguese cookies documentation with Ruby examples
+21/-81 
cookies.zh-cn.md
Update Chinese cookies documentation with Ruby examples   
+25/-81 
frames.en.md
Update English frames documentation with Ruby examples     
+22/-31 
frames.ja.md
Update Japanese frames documentation with Ruby examples   
+28/-20 
frames.pt-br.md
Update Portuguese frames documentation with Ruby examples
+21/-37 
frames.zh-cn.md
Update Chinese frames documentation with Ruby examples     
+20/-41 
Configuration changes
1 files
hugo.toml
Adjust Hugo configuration to ignore specific errors           
+1/-1     

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 Mar 21, 2025

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit b56ce2f

    Copy link
    Contributor

    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

    Potential Resource Leak

    The test creates a driver but doesn't quit it in the main test case. Unlike the cookies_spec.rb which has before/after hooks to manage the driver lifecycle, this test creates the driver inside the test but only quits it at the end without using a ensure/rescue block.

    driver = Selenium::WebDriver.for :chrome
    driver.manage.timeouts.implicit_wait = 0.5
    
    # Navigate to URL
    driver.get('https://www.selenium.dev/selenium/web/iframes.html')
    
    # Switch to iframe using WebElement
    iframe = driver.find_element(id: 'iframe1')
    driver.switch_to.frame(iframe)
    expect(driver.page_source.include?('We Leave From Here')).to be true
    
    # Interact with email field
    email_element = driver.find_element(id: 'email')
    email_element.send_keys('admin@selenium.dev')
    email_element.clear
    driver.switch_to.default_content
    
    # Switch to iframe using name
    driver.find_element(name: 'iframe1-name')
    driver.switch_to.frame(iframe)
    expect(driver.page_source.include?('We Leave From Here')).to be true
    
    email = driver.find_element(id: 'email')
    email.send_keys('admin@selenium.dev')
    email.clear
    driver.switch_to.default_content
    
    # Switch to iframe using index
    driver.switch_to.frame(0)
    expect(driver.page_source.include?('We Leave From Here')).to be true
    
    # Leave frame
    driver.switch_to.default_content
    expect(driver.page_source.include?('This page has iframes')).to be true
    
    # Quit the browser
    driver.quit
    Debugging Code

    There are debug print statements in the SameSite cookies test that should be removed before merging to production. These statements output cookie information but aren't part of any assertions.

    puts cookie_strict[:same_site]
    puts cookie_lax[:same_site]

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Fix frame switching logic

    The code attempts to find an element by name but doesn't use the result.
    Instead, it reuses the previously defined iframe variable. This could lead to
    unexpected behavior as the frame being switched to may not be the one that was
    just found.

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

    -driver.find_element(name: 'iframe1-name')
    -driver.switch_to.frame(iframe)
    +iframe_by_name = driver.find_element(name: 'iframe1-name')
    +driver.switch_to.frame(iframe_by_name)
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: The code finds an element by name but doesn't use it, instead reusing the previous iframe variable. This is a logical error that could cause the test to interact with the wrong frame, potentially leading to test failures.

    Medium
    Fix cookie deletion test

    In the "deletes a cookie using cookie object" test, the test is creating a
    cookie with name 'test2', but the test description suggests it should be
    deleting a cookie using a cookie object. However, it's actually deleting by
    name, not using the cookie object that was created.

    examples/ruby/spec/interactions/cookies_spec.rb [50-51]

    -@driver.manage.delete_cookie('test2')
    +cookie = { name: 'test2', value: 'cookie2' }
    +@driver.manage.add_cookie(cookie)
    +@driver.manage.delete_cookie(cookie)
     expect(@driver.manage.cookie_named('test2')).to be_nil
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    __

    Why: The test claims to delete a cookie using a cookie object but actually deletes by name. This misalignment between test description and implementation could cause confusion and doesn't properly test the intended functionality.

    Medium
    General
    Maintain consistent test URLs

    The test is using 'http://www.example.com' for cookie testing, but all other
    tests use 'https://www.selenium.dev/selenium/web/blank.html'. Using a different
    domain could cause inconsistency in test behavior, especially for cookie-related
    tests.

    examples/ruby/spec/interactions/cookies_spec.rb [63]

    -@driver.navigate.to 'http://www.example.com'
    +@driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    __

    Why: Using a different domain (example.com) for SameSite cookie tests while other tests use selenium.dev could lead to inconsistent behavior. Standardizing on one test URL improves test reliability and maintainability.

    Low
    • More

    @pallavigitwork pallavigitwork self-assigned this Mar 21, 2025
    @pallavigitwork
    Copy link
    Member Author

    @aguspe , thanks for helping with the PR. will wait for further information.

    @pallavigitwork
    Copy link
    Member Author

    halting work on this PR, as i didn't had the devtools gem installed. trying again.
    if it works will close this PR.

    @@ -3,5 +3,81 @@
    require 'spec_helper'

    RSpec.describe 'Cookies' do
    let(:driver) { start_session }
    Copy link
    Member

    Choose a reason for hiding this comment

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

    Do not use an instance variable in the specs.
    Do not start and stop a driver outside of the spec_helper.rb.

    keep start_session to define which session you need (in this case the default works)

    Copy link
    Member Author

    Choose a reason for hiding this comment

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

    Ok @titusfortner , this isn't the file i added. I added this

    require 'selenium-webdriver'
    require 'rspec'

    RSpec.describe 'CookiesTest' do
    before(:each) do
    @driver = Selenium::WebDriver.for :chrome
    end

    after(:each) do
    @driver.quit
    end

    it 'adds a cookie' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    @driver.manage.add_cookie(name: 'key', value: 'value')
    end

    it 'gets a named cookie' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    @driver.manage.add_cookie(name: 'foo', value: 'bar')
    cookie = @driver.manage.cookie_named('foo')
    expect(cookie[:value]).to eq('bar')
    end

    it 'gets all cookies' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    @driver.manage.add_cookie(name: 'test1', value: 'cookie1')
    @driver.manage.add_cookie(name: 'test2', value: 'cookie2')

    cookies = @driver.manage.all_cookies
    test1 = cookies.find { |c| c[:name] == 'test1' }
    test2 = cookies.find { |c| c[:name] == 'test2' }
    
    expect(test1[:value]).to eq('cookie1')
    expect(test2[:value]).to eq('cookie2')
    

    end

    it 'deletes a cookie by name' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    @driver.manage.add_cookie(name: 'test1', value: 'cookie1')
    @driver.manage.delete_cookie('test1')
    expect(@driver.manage.cookie_named('test1')).to be_nil
    end

    it 'deletes a cookie using cookie object' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    cookie = { name: 'test2', value: 'cookie2' }
    @driver.manage.add_cookie(cookie)
    @driver.manage.delete_cookie('test2')
    expect(@driver.manage.cookie_named('test2')).to be_nil
    end

    it 'deletes all cookies' do
    @driver.navigate.to 'https://www.selenium.dev/selenium/web/blank.html'
    @driver.manage.add_cookie(name: 'test1', value: 'cookie1')
    @driver.manage.add_cookie(name: 'test2', value: 'cookie2')
    @driver.manage.delete_all_cookies
    expect(@driver.manage.all_cookies).to be_empty
    end

    it 'creates SameSite cookies' do
    @driver.navigate.to 'http://www.example.com'

    cookie_strict = {
      name: 'key',
      value: 'value',
      same_site: 'Strict'
    }
    
    cookie_lax = {
      name: 'key',
      value: 'value',
      same_site: 'Lax'
    }
    
    @driver.manage.add_cookie(cookie_strict)
    @driver.manage.add_cookie(cookie_lax)
    
    puts cookie_strict[:same_site]
    puts cookie_lax[:same_site]
    

    end
    end

    thanks for letting me know. I have halted work on this one. will come back to it later.

    @pallavigitwork
    Copy link
    Member Author

    Closing this PR, will fix the code, environment as per information recvd after 2236 is resolved.

    @pallavigitwork pallavigitwork deleted the cookie-fix-pal branch April 21, 2025 12:10
    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.

    2 participants