[js] Fix failing javascript tests

[asolntsev]

🔗 Related Issues

e.g. https://github.com/SeleniumHQ/selenium/actions/runs/23901428682/job/69699326193?pr=17287

💥 What does this PR do?

Fixes the failing JS test.

🔄 Types of changes

• Bug fix (backwards compatible)

[qodo-code-review]

Review Summary by Qodo

Fix failing JavaScript tests and remove unnecessary delays

🐞 Bug fix 🧪 Tests

Walkthroughs

Description

• Fix flaky "can unpin script" test by checking script presence instead of exact count
• Remove unnecessary delays in test assertions that already wait via driver.get()
• Improve test robustness for Firefox which may call pinned scripts multiple times
• Refactor test to verify script pinning/unpinning behavior more reliably

File Changes

1. javascript/selenium-webdriver/test/lib/webdriver_script_test.js 🐞 Bug fix +9/-15

Fix flaky tests and remove unnecessary delays

• Removed unused delay() helper function that was causing test flakiness
• Removed three unnecessary 3-second delays from test assertions
• Refactored "can unpin script" test to check for script presence in logs instead of exact call
 count
• Changed assertion logic to use logs.includes() for more reliable cross-browser compatibility
• Updated pinned script output strings for clarity in test verification

javascript/selenium-webdriver/test/lib/webdriver_script_test.js

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. JS error race 🐞 Bug ☼ Reliability

Description

can listen to javascript error asserts on log.text immediately after the click even though the
JavaScript exception entry is delivered asynchronously, so log can still be null and the test can
fail/flap. This was previously masked by await delay(3000) which has been removed.

Code

javascript/selenium-webdriver/test/lib/webdriver_script_test.js[R63-68]

        await driver.get(Pages.logEntryAdded)
        await driver.findElement({ id: 'jsException' }).click()

-        await delay(3000)
-
        assert.equal(log.text, 'Error: Not working')
        assert.equal(log.type, 'javascript')
        assert.equal(log.level, 'error')

Evidence

addJavaScriptErrorHandler registers a LogInspector callback, and LogInspector invokes callbacks
from a WebSocket 'message' handler. Nothing in click()/get() awaits the log event, so the
immediate assertion can run before the callback fires.

javascript/selenium-webdriver/test/lib/webdriver_script_test.js[57-71]
javascript/selenium-webdriver/lib/script.js[54-62]
javascript/selenium-webdriver/bidi/logInspector.js[202-228]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The test asserts on `log` without waiting for the asynchronous BiDi log delivery.

### Issue Context
`onJavascriptException` processes events in a `ws.on('message', ...)` callback, so `log` is updated asynchronously.

### Fix Focus Areas
- javascript/selenium-webdriver/test/lib/webdriver_script_test.js[57-71]

### Suggested change
After clicking `#jsException`, wait until the handler runs, e.g.:
- `await driver.wait(() => log && log.text === 'Error: Not working', 5000)`
Then run the assertions and remove the handler.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Unpin test log races 🐞 Bug ☼ Reliability

Description

The pin/unpin tests assert on log/logs immediately after navigation and also clear logs
between navigations without ensuring all prior messages were received, so late-arriving entries from
the first navigation can be counted in the second phase. This can cause nondeterministic failures,
especially in the negative assertion that Hello is absent.

Code

javascript/selenium-webdriver/test/lib/webdriver_script_test.js[R126-150]

        await driver.get(Pages.logEntryAdded)

-        await delay(3000)
-
        assert.equal(log.text, 'Hello!')
      })

      it('can unpin script', async function () {
-        const id = await driver.script().pin("() => { console.log('Hello!'); }")
+        const id = await driver.script().pin("() => { console.log('Hello'); }")
+        await driver.script().pin("() => { console.log('World'); }")

-        let count = 0
+        const logs = []
        await driver.script().addConsoleMessageHandler((logEntry) => {
-          count++
+          logs.push(logEntry.text)
        })

        await driver.get(Pages.logEntryAdded)
+        assert.ok(logs.includes('Hello'), `[${logs}] should contain "Hello"`)
+        assert.ok(logs.includes('World'), `[${logs}] should contain "World"`)

        await driver.script().unpin(id)

+        logs.length = 0
        await driver.get(Pages.logEntryAdded)
-
-        assert.equal(count, 1)
+        assert.ok(logs.includes('World'), `[${logs}] should contain "World"`)
+        assert.ok(!logs.includes('Hello'), `[${logs}] should not contain "Hello"`)
      })

Evidence

Log callbacks are invoked asynchronously from a shared WebSocket 'message' handler. Clearing
logs does not stop already-triggered-but-not-yet-delivered messages from being pushed after the
reset, so phase 2 can observe messages originating from phase 1.

javascript/selenium-webdriver/test/lib/webdriver_script_test.js[118-150]
javascript/selenium-webdriver/bidi/logInspector.js[104-152]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The tests assume logs are available immediately after `driver.get()`, and the unpin test reuses the same handler across two navigations while clearing the array mid-stream.

### Issue Context
BiDi log delivery is asynchronous (`ws.on('message')`). Without waiting, assertions can run too early; and without isolating phases, late entries from navigation 1 can be observed after `logs.length = 0` during navigation 2.

### Fix Focus Areas
- javascript/selenium-webdriver/test/lib/webdriver_script_test.js[118-150]

### Suggested change
1) For `can pin script`, after `driver.get(...)` wait until `log` is populated:
- `await driver.wait(() => log && log.text === 'Hello!', 5000)`

2) For `can unpin script`:
- Capture the handler id returned by `addConsoleMessageHandler`.
- After the first `driver.get(...)`, `await driver.wait(() => logs.includes('Hello') && logs.includes('World'), 5000)` before asserting.
- Remove the handler (`removeConsoleMessageHandler(handlerId)`) and add a fresh handler for phase 2 (or otherwise gate messages) to prevent late messages from phase 1 being recorded.
- After the second `driver.get(...)`, `await driver.wait(() => logs.includes('World'), 5000)` and then assert `!logs.includes('Hello')`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more