[js] Migrate javascript/atoms tests from Closure to QUnit

[shs96c]

User description

The Closure testing framework is being phased out in favor of QUnit, which provides a more modern and maintainable test infrastructure. Later PRs will migrate the other usages of the Closure testing library.

Several tests required DOM isolation via iframes because QUnit injects its own UI elements (#qunit, #qunit-fixture) that interfere with tests that count or query DOM elements.

The Java test shim was updated to detect both Closure and QUnit test runners.

---

PR Type

Tests, Enhancement

---

Description

This PR migrates JavaScript atoms tests from the deprecated Closure testing framework to QUnit, providing a more modern and maintainable test infrastructure.

Key Changes:

• Migrated 30+ test files from Closure's goog.testing.jsunit to QUnit test framework

• Converted all test functions from Closure format (function testName()) to QUnit format (QUnit.test())

• Replaced Closure assertions (assertEquals, assertTrue, assertFalse) with QUnit equivalents (assert.strictEqual, assert.ok, assert.notOk)

• Implemented iframe-based test isolation for DOM-sensitive tests to prevent QUnit UI interference

• Updated Java test harness (ClosureTestStatement.java) to detect and support both Closure and QUnit test runners

• Created QUnit test runner adapter (qunit_test_runner.js) for integration with Selenium's Java test infrastructure

• Added QUnit 2.23.1 library to third-party dependencies with Bazel build configuration

• Converted async tests to use QUnit's assert.async() pattern

• Replaced ExpectedFailures pattern with inline skip logic for browser-specific tests

• Updated test lifecycle hooks from Closure's setUp/tearDown to QUnit's testStart/testDone

• Modified helper functions throughout to accept QUnit's assert parameter

Test Files Migrated:

• Core functionality: click_test.html, mouse_test.html, keyboard_test.html, touchscreen_test.html, type_test.html

• DOM interaction: locator_test.html, relative_locator_test.html, select_test.html, text_test.html, style_test.html

• Element state: shown_test.html, enabled_test.html, overflow_test.html, scrolling_test.html

• HTML5 features: database_test.html, storage_test.html, location_test.html, shadow_dom_test.html

• And many more test suites covering various atoms functionality

---

Diagram Walkthrough

flowchart LR
  A["Closure Test Framework"] -- "migrate" --> B["QUnit Framework"]
  B --> C["30+ Test Files"]
  B --> D["Java Test Harness"]
  D --> E["ClosureTestStatement.java"]
  E -- "detects" --> F["TestRunner enum"]
  F --> G["Closure Runner"]
  F --> H["QUnit Runner"]
  H --> I["qunit_test_runner.js"]
  B --> J["QUnit 2.23.1 Library"]
  J --> K["Bazel Build Config"]
  C --> L["Iframe Isolation"]
  C --> M["Async Test Support"]

Loading

File Walkthrough

	Relevant files
Enhancement	2 files ClosureTestStatement.java Add dual test runner support for Closure and QUnit frameworks java/test/org/openqa/selenium/javascript/ClosureTestStatement.java Refactored test runner detection to support both Closure and QUnit frameworks Added detectTestRunner() method that polls for test runner initialization Converted Query enum to TestRunner enum with separate implementations for Closure and QUnit Updated cleanup logic to clear both G_testRunner and QUnitTestRunner between tests +78/-22  qunit_test_runner.js Implement QUnit test runner adapter for Java harness integration third_party/js/qunit/qunit_test_runner.js Created new QUnit test runner adapter for Selenium's Java test harness Exposes QUnitTestRunner interface on window.top with isFinished(), isSuccess(), and getReport() methods Registers QUnit callbacks to track test execution and collect failure details Formats test results report with counts, runtime, and detailed failure information +99/-0
Tests	18 files useragent_test.js Migrate useragent tests from Closure to QUnit framework    javascript/atoms/test/useragent_test.js Migrated all test functions from Closure's function testName() format to QUnit's QUnit.test() format Replaced Closure assertions (assertTrue, assertFalse) with QUnit assertions (assert.ok, assert.notOk) Added conditional skip logic for browser-specific tests using assert.ok(true, 'Skipping: ...') +34/-30  text_util.js Adapt text utility helpers for QUnit assertion style          javascript/atoms/test/text_util.js Updated assertTextIs() helper function to accept QUnit assert object and testName parameters Replaced Closure's assertEquals() with QUnit's assert.strictEqual() Removed dependency on goog.testing.TestCase for accessing current test name +7/-7      locator_test.html Migrate locator tests to QUnit with iframe isolation          javascript/atoms/test/locator_test.html Converted from Closure test format to QUnit with proper HTML structure and QUnit UI elements Migrated all test functions to QUnit.test() format with QUnit assertions Implemented iframe-based test isolation to prevent QUnit UI interference with DOM tests Added test lifecycle hooks (QUnit.begin, QUnit.testStart, QUnit.testDone) for setup/teardown Embedded test page HTML directly in iframe instead of loading from separate file +518/-433 inject_test.html Convert inject tests from Closure to QUnit framework          javascript/atoms/test/inject_test.html Migrated all test functions from Closure to QUnit format Replaced Closure assertions with QUnit equivalents (assert.strictEqual, assert.ok, assert.throws, etc.) Converted promise-based async tests to use QUnit's assert.async() pattern Removed goog.testing.jsunit dependency and added QUnit script includes +296/-308 click_link_test.html Migrate click link tests to QUnit with async support          javascript/atoms/test/click_link_test.html Migrated all test functions to QUnit format with proper async handling using assert.async() Replaced Closure assertions with QUnit assertions throughout Refactored test helper functions to accept assert parameter Added QUnit UI elements and removed goog.testing.jsunit dependency Updated async test patterns to use QUnit's callback-based approach instead of promises +327/-260 response_test.html Migrate response tests from Closure to QUnit                          javascript/atoms/test/response_test.html Converted all test functions from Closure format to QUnit's QUnit.test() format Replaced assertEquals() and assertTrue() with assert.strictEqual() and assert.ok() Added QUnit HTML structure and script includes +24/-19  select_test.html Migrate select_test.html from Closure to QUnit testing framework javascript/atoms/test/select_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Replaced setUpPage() and setUp() with QUnit's testStart() hook Converted all test functions from Closure format to QUnit's QUnit.test() format Updated assertion methods from Closure style (assertEquals, assertTrue, etc.) to QUnit style (assert.strictEqual, assert.ok, etc.) Added QUnit UI elements (#qunit, #qunit-fixture) to HTML body Removed goog.require('goog.testing.jsunit') and added QUnit script/stylesheet references +266/-243 mouse_test.html Migrate mouse_test.html from Closure to QUnit testing framework javascript/atoms/test/mouse_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Replaced setUpPage() and setUp() with QUnit's testStart() hook and module-level initialization Converted all test functions to QUnit's QUnit.test() format with assert parameter Updated assertion methods to QUnit style and modified assertEvents helper to accept assert parameter Added async test support using assert.async() for promise-based tests Added QUnit UI elements and removed Closure testing dependencies +117/-132 shown_test.html Migrate shown_test.html from Closure to QUnit testing framework javascript/atoms/test/shown_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Replaced setUp() and tearDown() with QUnit's testDone() hook Converted all test functions to QUnit's QUnit.test() format Replaced ExpectedFailures pattern with inline skip logic using assert.ok(true, 'Skipping...') Updated all assertions to QUnit style (assert.ok, assert.notOk, assert.strictEqual) Added QUnit UI elements and removed Closure testing dependencies +204/-221 type_test.html Migrate type_test.html from Closure to QUnit testing framework javascript/atoms/test/type_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Replaced setUp() and tearDown() with QUnit's testStart() hook Converted all test functions to QUnit's QUnit.test() format Updated runTypingTest helper function to accept assert parameter Replaced ExpectedFailures with inline skip logic and converted async tests to use assert.async() Updated all assertions to QUnit style and added QUnit UI elements +153/-209 text_test.html Migrate text_test.html from Closure to QUnit testing framework javascript/atoms/test/text_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Converted all test functions to QUnit's QUnit.test() format Updated custom assertion helpers (assertContains, assertStartsWith, assertEndsWith) to accept assert parameter Replaced ExpectedFailures pattern with inline skip logic Updated all assertions to QUnit style (assert.strictEqual, assert.ok) Added QUnit UI elements and removed Closure testing dependencies +162/-158 style_test.html Migrate style tests from Closure to QUnit framework            javascript/atoms/test/style_test.html Migrated from Closure's goog.testing.jsunit to QUnit test framework Replaced assertEquals, assertTrue, assertContains with QUnit's assert.strictEqual, assert.ok Converted global test functions to QUnit.test() calls with descriptive names Added QUnit UI elements (#qunit, #qunit-fixture) to HTML body Introduced custom assertColorFuzzyEquals helper that accepts assert parameter Removed duplicate test function definitions +111/-209 click_test.html Convert click action tests to QUnit test framework              javascript/atoms/test/click_test.html Converted from Closure jsunit to QUnit test framework Replaced setUp/tearDown with QUnit.testStart/QUnit.testDone hooks Changed assertion methods from Closure style to QUnit (assert.strictEqual, assert.deepEqual, assert.throws) Refactored helper functions to accept assert parameter and use QUnit assertions Updated coordinate comparison to use assert.strictEqual instead of custom assertEquals Added QUnit UI container divs to HTML body +142/-130 relative_locator_test.html Migrate relative locator tests to QUnit with iframe isolation javascript/atoms/test/relative_locator_test.html Migrated from Closure testing framework to QUnit Moved test page HTML into iframe to isolate DOM from QUnit UI elements Added QUnit.begin, QUnit.testStart, QUnit.testDone lifecycle hooks for iframe setup/teardown Converted all test functions to QUnit.test() with proper assertions Replaced assertEquals, assertTrue with assert.strictEqual, assert.ok Moved CSS styles into dynamically generated test page source +155/-222 overflow_test.html Convert overflow state tests to QUnit framework                    javascript/atoms/test/overflow_test.html Converted from Closure jsunit to QUnit test framework Replaced tearDown with QUnit.testDone hook Changed all assertion methods to QUnit equivalents (assert.strictEqual, assert.notOk) Converted test functions to QUnit.test() calls with descriptive names Added conditional test skipping with assert.ok(true, 'Skipping...') for unsupported browsers Added QUnit UI elements to HTML body +107/-119 touchscreen_test.html Migrate touchscreen interaction tests to QUnit framework  javascript/atoms/test/touchscreen_test.html Migrated from Closure testing framework to QUnit Replaced setUp with QUnit.testStart and added QUnit.begin for page setup Converted all test functions to QUnit.test() with QUnit assertions Updated helper functions (assertEvents, verifyTapEvents) to accept assert parameter Changed assertions from Closure style to QUnit (assert.ok, assert.strictEqual, assert.deepEqual) Added conditional test skipping for unsupported touch events Made async test properly use assert.async() for scroll testing +128/-103 scrolling_test.html Convert scrolling behavior tests to QUnit framework            javascript/atoms/test/scrolling_test.html Converted from Closure jsunit to QUnit test framework Replaced setUp/tearDown with QUnit.testStart/QUnit.testDone hooks Removed goog.testing.ExpectedFailures in favor of conditional test skipping Changed all assertions to QUnit equivalents (assert.strictEqual, assert.notStrictEqual, assert.ok) Converted test functions to QUnit.test() with descriptive names Added QUnit UI container divs to HTML body +116/-127 text_table_test.html Migrate table text extraction tests to QUnit framework      javascript/atoms/test/text_table_test.html Migrated from Closure jsunit to QUnit test framework Updated setUp/tearDown to QUnit.testStart/QUnit.testDone hooks Modified assertTextIs helper to accept assert parameter Converted all test functions to QUnit.test() with descriptive test names Retained goog.testing.ExpectedFailures for browser-specific failure handling Added QUnit UI elements to HTML body +68/-62
Configuration changes	2 files test_suite.bzl Include QUnit library in test suite dependencies                  javascript/private/test_suite.bzl Added //third_party/js/qunit to the default data dependencies for closure test suites +1/-0      BUILD.bazel Add Bazel build configuration for QUnit library                    third_party/js/qunit/BUILD.bazel Created new Bazel build file for QUnit library dependencies Defined filegroup target containing QUnit CSS, JS, and test runner files Set visibility to allow access from Java test environment and JavaScript subpackages +14/-0
Documentation	1 files VERSION Document QUnit version 2.23.1                                                        third_party/js/qunit/VERSION Added version file indicating QUnit 2.23.1 is being used +1/-0
Additional files	42 files action_test.html +142/-124 attribute_test.html +47/-43  child_locator_test.html +90/-86  click_submit_test.html +105/-82 clientrect_test.html +54/-57  color_test.html +31/-36  dom_test.html +21/-18  drag_test.html +26/-36  enabled_test.html +45/-41  enter_submit_test.html +49/-34  error_test.html +17/-6    events_test.html +56/-59  focus_test.html +62/-65  frame_scroll_test.html +20/-15  frame_size_test.html +23/-18  frame_test.html +87/-83  gestures_test.html +69/-49  history_test.html +67/-61  appcache_test.html +15/-5    database_test.html +167/-119 location_test.html +62/-48  nested_window_storage_test.html +26/-19  no_manifest_appcache_test.html +12/-8    shadow_dom_test.html +52/-37  storage_test.html +77/-60  interactable_size_test.html +22/-23  keyboard_test.html +110/-94 opacity_test.html +23/-21  orientation_test.html +36/-32  property_test.html +34/-29  svg_test.html +76/-50  text_box_test.html +45/-27  text_shadow_test.html +26/-23  toolbar_test.html +19/-14  useragent_quirks_test.html +9/-1      useragent_test.html +8/-1      window_scroll_into_view_test.html +48/-26  window_scroll_test.html +19/-12  window_size_test.html +28/-19  LICENSE +22/-0    qunit.css +525/-0  qunit.js +7339/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Stack trace exposure: The QUnit report construction includes err.stack, which may expose internal stack traces outside secure logs depending on how JavaScriptAssertionError is surfaced in CI artifacts. Referred Code QUnit.on('testEnd', function(testEnd) { if (testEnd.status === 'failed') { var fullName = testEnd.fullName.join(' > '); var errors = testEnd.errors.map(function(err) { var msg = err.message || ''; if (err.actual !== undefined && err.expected !== undefined) { msg += '\n Expected: ' + JSON.stringify(err.expected); msg += '\n Actual: ' + JSON.stringify(err.actual); } if (err.stack) { msg += '\n Stack: ' + err.stack; } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unredacted failure details: The failure report concatenates raw err.actual/err.expected and stack traces into a single unstructured string, which could inadvertently leak sensitive values into test logs/artifacts if such data appears in assertions. Referred Code var errors = testEnd.errors.map(function(err) { var msg = err.message || ''; if (err.actual !== undefined && err.expected !== undefined) { msg += '\n Expected: ' + JSON.stringify(err.expected); msg += '\n Actual: ' + JSON.stringify(err.actual); } if (err.stack) { msg += '\n Stack: ' + err.stack; } return msg; Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix incorrect variable capture in loop Fix a bug in the test by using an IIFE to correctly capture the toggleElem variable inside the loop's event listener, ensuring each iteration tests the correct element. javascript/atoms/test/select_test.html [554-566] for (var i = 0; i < toggleElems.length; ++i) { - var toggleElem = toggleElems[i]; - var preState = bot.dom.isSelected(toggleElem); - var postState = null; - goog.events.listen(toggleElem, 'change', function(e) { - postState = bot.dom.isSelected(toggleElem); - }); - action(toggleElem); - assert.ok(postState !== null, 'change event did not fire'); - assert.notStrictEqual(postState, preState, 'element did not toggle'); + (function(toggleElem) { + var preState = bot.dom.isSelected(toggleElem); + var postState = null; + goog.events.listen(toggleElem, 'change', function(e) { + postState = bot.dom.isSelected(toggleElem); + }); + action(toggleElem); + assert.ok(postState !== null, 'change event did not fire'); + assert.notStrictEqual(postState, preState, 'element did not toggle'); + })(toggleElems[i]); } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a subtle but critical bug in the test logic due to incorrect variable capturing in a loop's asynchronous callback, which would cause the test to produce incorrect results.	High
	Remove tautology from test assertion Remove the || true from the assert.ok call in the shouldFindElementWithCommaInAttribute test to ensure the assertion is not always true and the test can fail correctly. javascript/atoms/test/locator_test.html [264-268] QUnit.test('shouldFindElementWithCommaInAttribute', function(assert) { assert.ok(bot.locators.findElement({css: 'comma-in-alt[alt="has, a comma"]'}) !== null || - bot.locators.findElement({css: '#comma-in-alt[alt="has, a comma"]'}) !== null || - true); + bot.locators.findElement({css: '#comma-in-alt[alt="has, a comma"]'}) !== null); }); Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a tautological condition (|| true) that causes the test to always pass. Fixing this is crucial to restore the test's intended validation, which was broken during the refactoring.	Medium
	Fix incorrect test logic implementation In the test 'tap on hash does not reload the page', replace the call to onSelfPageReloadsPage with actionOnHashDoesNotReloadThePage to align the test logic with its description. javascript/atoms/test/click_link_test.html [542-544] QUnit.test('tap on hash does not reload the page', function(assert) { - onSelfPageReloadsPage(assert, bot.action.tap); + actionOnHashDoesNotReloadThePage(assert, bot.action.tap); }); Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a significant copy-paste error where a test calls a helper function with the opposite logic of what the test name describes. This fix is critical to ensure the test validates the correct behavior.	Medium
	Reset counters before each test Prevent test state leakage by resetting changeCount, gotPointerCaptureCount, and lostPointerCaptureCount to zero within the QUnit.testStart hook. javascript/atoms/test/select_test.html [71-95] QUnit.testStart(function() { + changeCount = 0; + gotPointerCaptureCount = 0; + lostPointerCaptureCount = 0; singleSelect = bot.locators.findElement({ id: 'singleSelect' }); multiSelect = bot.locators.findElement({ id: 'multiSelect' }); checkboxes = bot.locators.findElements({ name: 'checkbox' }); radioButtons = bot.locators.findElements({ name: 'radio' }); - goog.events.removeAll(singleSelect); - goog.events.removeAll(multiSelect); - for (var i = 0; i < checkboxes.length; ++i) { - goog.events.removeAll(checkboxes[i]); - } - for (var i = 0; i < radioButtons.length; ++i) { - goog.events.removeAll(radioButtons[i]); - } - singleSelect.selectedIndex = 0; - goog.array.forEach(multiSelect.options, function (e) { - e.selected = false; - }); - goog.array.forEach(checkboxes, function (e) { - e.checked = false; - }); - goog.array.forEach(radioButtons, function (e) { - e.checked = false; - }); -}); - Apply / Chat Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies that state from previous tests can leak into subsequent ones, which is a critical issue in testing, and proposes resetting the counters to ensure test isolation.	Medium
	Prevent crashes from circular references Wrap JSON.stringify calls in a try...catch block to handle potential TypeError from circular references in error objects, falling back to String(value) on failure. third_party/js/qunit/qunit_test_runner.js [51-61] var errors = testEnd.errors.map(function(err) { var msg = err.message || ''; if (err.actual !== undefined && err.expected !== undefined) { - msg += '\n Expected: ' + JSON.stringify(err.expected); - msg += '\n Actual: ' + JSON.stringify(err.actual); + try { + msg += '\n Expected: ' + JSON.stringify(err.expected); + msg += '\n Actual: ' + JSON.stringify(err.actual); + } catch (e) { + msg += '\n Expected: ' + String(err.expected); + msg += '\n Actual: ' + String(err.actual); + } } if (err.stack) { msg += '\n Stack: ' + err.stack; } return msg; }); [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies that JSON.stringify can fail on circular references and proposes a robust try...catch block to handle such errors, preventing the test runner from crashing and improving the reliability of test reporting.	Medium
	Use Boolean cast for script results Change the type of hasQUnit and hasClosure from boolean to Boolean and use Boolean.TRUE.equals() for null-safe checks to prevent potential NullPointerException. java/test/org/openqa/selenium/javascript/ClosureTestStatement.java [104-113] -boolean hasQUnit = - (boolean) executor.executeScript("return !!(window.top.QUnitTestRunner || window.QUnit);"); +Boolean hasQUnit = + (Boolean) executor.executeScript("return !!(window.top.QUnitTestRunner || window.QUnit);"); +if (Boolean.TRUE.equals(hasQUnit)) { + LOG.fine("Detected QUnit test runner"); + return TestRunner.QUNIT; +} ... -boolean hasClosure = - (boolean) executor.executeScript("return !!window.top.G_testRunner;"); +Boolean hasClosure = + (Boolean) executor.executeScript("return !!window.top.G_testRunner;"); +if (Boolean.TRUE.equals(hasClosure)) { + LOG.fine("Detected Closure test runner"); + return TestRunner.CLOSURE; +} [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies that casting a potential null return from executeScript to a primitive boolean can cause a NullPointerException. The proposed change to use the Boolean wrapper class is a valid and important improvement for robustness.	Medium
General	Guard QUnit event registration Wrap the QUnit.on() event registrations in a check for typeof QUnit !== 'undefined' && typeof QUnit.on === 'function' to prevent errors if the script loads before QUnit. third_party/js/qunit/qunit_test_runner.js [48-98] -QUnit.on('testEnd', function(testEnd) { - // ... -}); -QUnit.on('runEnd', function(runEnd) { - // ... -}); +if (typeof QUnit !== 'undefined' && typeof QUnit.on === 'function') { + QUnit.on('testEnd', function(testEnd) { + // ... + }); + QUnit.on('runEnd', function(runEnd) { + // ... + }); +} [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 6 __ Why: This is a good defensive programming practice. It correctly identifies that the adapter script could potentially run before QUnit is initialized, and adding a guard prevents runtime errors, making the test runner more robust against race conditions.	Low
	Narrow QUnit detection script Simplify the QUnit detection script to only check for window.top.QUnitTestRunner, ensuring the adapter is loaded before selecting the QUnit runner. java/test/org/openqa/selenium/javascript/ClosureTestStatement.java [106] -executor.executeScript("return !!(window.top.QUnitTestRunner || window.QUnit);"); +executor.executeScript("return !!window.top.QUnitTestRunner;"); Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that checking for window.top.QUnitTestRunner is sufficient and more precise, as this interface is what the test harness polls. This simplifies the detection logic and makes it more aligned with the implementation.	Low
	Use assert.throws for exception testing Refactor the test to use QUnit's assert.throws() method for exception testing instead of a manual try...catch block, making the code more concise and idiomatic. javascript/atoms/test/shown_test.html [134-144] QUnit.test('throws error if passed a text node', function(assert) { var textNode = findElement({ id: 'displayed' }).firstChild; - var err = null; - try { - isShown(textNode); - } catch (e) { - err = e; - } - assert.ok(err !== null, 'Argument to isShown must be of type Element'); - assert.strictEqual(err.message, 'Argument to isShown must be of type Element'); + assert.throws( + function() { + isShown(textNode); + }, + /Argument to isShown must be of type Element/, + 'should throw an error for text nodes' + ); }); Apply / Chat Suggestion importance[1-10]: 5 __ Why: The suggestion correctly proposes using assert.throws to simplify the test and improve readability, which is a good practice, but it overlaps with another suggestion targeting the same code.	Low
Learned best practice	Ensure iframe teardown always runs Add a QUnit.done (or QUnit.module(..., hooks)) teardown that restores bot window state and removes testFrame even if tests fail mid-run. javascript/atoms/test/locator_test.html [87-100] QUnit.begin(function() { testFrame = document.createElement('iframe'); testFrame.setAttribute('width', 800); testFrame.setAttribute('height', 600); testFrame.setAttribute('id', 'testFrame'); document.body.appendChild(testFrame); var win = goog.dom.getFrameContentWindow(testFrame); win.document.open(); win.document.write(TEST_PAGE_SOURCE); win.document.close(); originalWindow = bot.getWindow(); }); +QUnit.done(function() { + try { + if (originalWindow) { + bot.setWindow(originalWindow); + } + } finally { + if (testFrame && testFrame.parentNode) { + testFrame.parentNode.removeChild(testFrame); + } + } +}); + Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - In tests that create external resources (iframes/contexts), always clean up in a guaranteed teardown path to avoid leaking resources across failures.	Low
Update

[titusfortner]

I like the approach of keeping test and lib updates independent. Qunit does look like the right choice for this. Runner toggle looks good.