Skip to content

Moved javascript code examples for element locators into test files #2312

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 4 commits into
base: trunk
Choose a base branch
from

Conversation

SuperDXCEL
Copy link

@SuperDXCEL SuperDXCEL commented May 18, 2025

User description

Description

This PR moves all JavaScript code examples for the following element locators into test files and updates the corresponding documentation to reference those examples using gh-codeblock:

className

cssSelector

id

name

tagName

xpath

linkText

partialLinkText

This ensures all locator examples are tested automatically and reduces duplication.
Unfortunately I could not get hugo working with my PAT so I could not preview the changes on the website directly

P.S: This is my first pull request, I really like Selenium so any tips you have would be greatly appreciated!

Motivation and Context

This PR helps integrate the documentation examples for the locators into CI

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


Description

  • Added comprehensive JavaScript tests for all element locator strategies.

    • Covers className, cssSelector, id, name, tagName, xpath, linkText, partialLinkText.
  • Updated documentation to reference tested code examples via gh-codeblock.

    • Ensures documentation examples are always tested and up-to-date.
  • Fixed minor typo in checkbox test comment.


Changes walkthrough 📝

Relevant files
Tests
locators.spec.js
Add JavaScript tests for all element locator strategies   

examples/javascript/test/elements/locators.spec.js

  • Added new test suite for element locator strategies in JavaScript.
  • Includes tests for className, cssSelector, id, name, tagName, xpath,
    linkText, and partialLinkText.
  • Each test verifies element identification and relevant attributes.
  • +118/-0 
    Documentation
    locators.en.md
    Reference tested JavaScript locator examples in documentation

    website_and_docs/content/documentation/webdriver/elements/locators.en.md

  • Updated JavaScript code examples to use gh-codeblock referencing new
    test file.
  • Ensures documentation examples are sourced from tested code.
  • Removed inline JavaScript code snippets for locators.
  • +8/-16   
    Miscellaneous
    information.spec.js
    Fix typo and formatting in element information test           

    examples/javascript/test/elements/information.spec.js

  • Fixed typo in checkbox test comment ("ins" to "is").
  • Minor formatting adjustment (newline at end of file).
  • +2/-2     

    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 May 18, 2025

    👷 Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    🔨 Latest commit 09f6579
    @CLAassistant
    Copy link

    CLAassistant commented May 18, 2025

    CLA assistant check
    All committers have signed the CLA.

    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

    XPath Mismatch

    The XPath example in the documentation references a female radio button with value 'f', but the test uses a newsletter checkbox with a different XPath. This inconsistency should be resolved.

    {{< gh-codeblock path="examples/javascript/test/elements/locators.spec.js#L66-L68" >}}
      {{< /tab >}}
    Copy link
    Contributor

    qodo-merge-pro bot commented May 18, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Ensure driver cleanup

    The test creates a new WebDriver instance for each test but doesn't properly
    handle cleanup if an assertion fails. Wrap the test logic in a try/finally block
    to ensure the driver is always quit, even when assertions fail.

    examples/javascript/test/elements/locators.spec.js [65-68]

     it('Check if element can be found by xpath', async function () {
       let driver = await new Builder().forBrowser('chrome').build();
    -  await driver.get('https://www.selenium.dev/selenium/web/locators_tests/locators.html');
    -  const element = await driver.findElement(By.xpath('//input[@name="newsletter"]'));
    +  try {
    +    await driver.get('https://www.selenium.dev/selenium/web/locators_tests/locators.html');
    +    const element = await driver.findElement(By.xpath('//input[@name="newsletter"]'));
     
    +    const tag = await element.getTagName();
    +    const type = await element.getAttribute("type");
    +    const value = await element.getAttribute("value");
    +
    +    assert.equal(tag, "input");
    +    assert.equal(type, "checkbox");
    +    assert.equal(value, "1");
    +  } finally {
    +    await driver.quit();
    +  }
    +});
    +

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 7

    __

    Why: Wrapping the test logic in a try/finally block to guarantee driver cleanup is a good practice that improves test reliability and prevents resource leaks, but it is a standard improvement rather than a critical fix.

    Medium
    General
    Add test timeout configuration

    The test suite lacks a timeout configuration, which can lead to test failures in
    slow environments or network issues. Add a Mocha timeout setting to allow
    sufficient time for browser operations.

    examples/javascript/test/elements/locators.spec.js [1-2]

     const {By, Builder} = require('selenium-webdriver');
     const assert = require("assert");
     
    +describe('Element Locator Test', function () {
    +  this.timeout(30000); // 30 seconds timeout for all tests
    +

    [To ensure code accuracy, apply this suggestion manually]

    Suggestion importance[1-10]: 6

    __

    Why: Adding a test timeout helps prevent false negatives in slow environments, improving test robustness, but it is a minor configuration enhancement rather than a major functional change.

    Low
    • Update
    @SuperDXCEL
    Copy link
    Author

    I could not add the try catch block in the test functions because gh-codeblock does not allow discontinuous lines. Because of this, I could not fit the declaration and definition of the webdriver object before the URL fetch and the relevant element fetch.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    3 participants