seleniumhq.github.io icon indicating copy to clipboard operation
seleniumhq.github.io copied to clipboard

Moved javascript code examples for element locators into test files

Open SuperDXCEL opened this issue 6 months ago • 5 comments
trafficstars

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

  • [x] 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.
  • SuperDXCEL avatar May 18 '25 21:05 SuperDXCEL

    Deploy request for selenium-dev pending review.

    Visit the deploys page to approve it

    Name Link
    Latest commit 317caa678a50e42dfe454e80d75b6fff6d389867

    netlify[bot] avatar May 18 '25 21:05 netlify[bot]

    CLA assistant check
    All committers have signed the CLA.

    CLAassistant avatar May 18 '25 21:05 CLAassistant

    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 >}}
    

    qodo-merge-pro[bot] avatar May 18 '25 21:05 qodo-merge-pro[bot]

    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

    qodo-merge-pro[bot] avatar May 18 '25 21:05 qodo-merge-pro[bot]

    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.

    SuperDXCEL avatar May 19 '25 09:05 SuperDXCEL