Skip to content
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

Infrastructure: Remove temp pinning to Node 16 to work around tests failing with Node 18 in CI #2636

Closed
howard-e opened this issue Mar 1, 2023 · 7 comments · Fixed by #2749
Labels
bug Code defects; not for inaccurate prose Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo

Comments

@howard-e
Copy link
Contributor

howard-e commented Mar 1, 2023

Initially reported by @jongund through an email thread and further supported by the latest set of dependabot updates, all the regression tests are failing with a similar error for each test, when ran with Node 18 in the GitHub Actions:

not ok 1 - accordion_accordion › before hook
  ---
    name: AssertionError
    message: Rejected promise returned by test
    values:
      'Rejected promise returned by test. Reason:': |-
        Error {
          message: 'Timed out while waiting for WebDriver server',
        }
    at: 'Timeout.poll [as _onTimeout] (test/util/start-geckodriver.js:34:16)'
  ...

The error message is Timed out while waiting for WebDriver server.

@howard-e howard-e added bug Code defects; not for inaccurate prose Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation labels Mar 1, 2023
@howard-e
Copy link
Contributor Author

howard-e commented Mar 2, 2023

The difference between the recent failing PRs and the passing PRs is the passing PRs were using node v16.x.x while the failing ones are now making use of node v18.x.x, as actions/setup-node@v3 is now using the latest LTS because we're not pinning the node version.

This issue was identified in SeleniumHQ/selenium#10970 (ISSUE 2). The builds should pass by merging #2635 as this bug was addressed in selenium-webdriver v4.5.0 but that dependabot PR is also failing and I'm unable to reproduce the failure locally.

@howard-e
Copy link
Contributor Author

howard-e commented Mar 2, 2023

May be potentially related to a regression with how child_process.spawn works in CI for node 18.

@howard-e howard-e changed the title Infrastructure: Failing regression tests Infrastructure: Failing regression tests with Node 18 in CI Mar 7, 2023
mcking65 pushed a commit that referenced this issue Mar 7, 2023
* Bump ava and selenium-webdriver
* (temporary fix tracked by #2636): Force node-version to 16 for regression.yml
@howard-e
Copy link
Contributor Author

howard-e commented Mar 7, 2023

The difference between the recent failing PRs and the passing PRs is the passing PRs were using node v16.x.x while the failing ones are now making use of node v18.x.x, as actions/setup-node@v3 is now using the latest LTS because we're not pinning the node version.

#2640 now pins the version back to v16 and is a temporary fix for this issue, while we investigate.

@mcking65 mcking65 changed the title Infrastructure: Failing regression tests with Node 18 in CI Infrastructure: Remove temp pinning to Node 16 to work around tests failing with Node 18 in CI Jun 18, 2023
@mcking65 mcking65 added the regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo label Jun 18, 2023
@mcking65
Copy link
Contributor

@howard-e, will the bugs associated with Node 18 effect only CI or could it also effect local execution of tests?

@howard-e
Copy link
Contributor Author

howard-e commented Jun 21, 2023

will the bugs associated with Node 18 effect only CI or could it also effect local execution of tests?

@mcking65 it only affects the tests when running in the GH CI from what I can see. At least, no unexpected errors have been thrown locally for me with node 18 on Windows and MacOS (with some failures due to specific key commands being referenced).

@howard-e
Copy link
Contributor Author

howard-e commented Jun 27, 2023

will the bugs associated with Node 18 effect only CI or could it also effect local execution of tests?

it only affects the tests when running in the GH CI from what I can see.

@mcking65 based on today's APG discussion around #2735, it seems I was wrong about this statement. After repeatedly running the regression tests with MacOS + Node 18 locally, I eventually got the same timeout error that's reported in the top comment of this PR (and it happens for all the tests).

Detailed regression tests results when ran on MacOS + Node 18:

✘ [fail]: breadcrumb_breadcrumb › before hook Rejected promise returned by test
✘ 2 tests remaining in test/tests/breadcrumb_breadcrumb.js
✘ [fail]: accordion_accordion › before hook Rejected promise returned by test
✘ 9 tests remaining in test/tests/accordion_accordion.js
...

breadcrumb_breadcrumb › before hook
Rejected promise returned by test. Reason:
Error {
message: 'Timed out while waiting for WebDriver server',
}
› Timeout.poll [as _onTimeout] (test/util/start-geckodriver.js:34:16)

accordion_accordion › before hook
Rejected promise returned by test. Reason:
Error {
message: 'Timed out while waiting for WebDriver server',
}
› Timeout.poll [as _onTimeout] (test/util/start-geckodriver.js:34:16)
...

For MacOS + Node 16, I'd consistently get the following, which is the same experience described in #2735:

5 tests failed
6 known failures

Detailed regression tests results when ran on MacOS + Node 16:

disclosure_navigation › content/patterns/disclosure/examples/disclosure-navigation.html [data-test-id="link-aria-current"]: "aria-current" attribute on links

Rejected promise returned by test. Reason:

ElementNotInteractableError {
remoteStacktrace: RemoteError@chrome://remote/content/shared/RemoteError.sys.mjs:8:8␊ WebDriverError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:183:5␊ ElementNotInteractableError@chrome://remote/content/shared/webdriver/Errors.sys.mjs:293:5␊ webdriverClickElement@chrome://remote/content/marionette/interaction.sys.mjs:150:11␊ interaction.clickElement@chrome://remote/content/marionette/interaction.sys.mjs:119:11␊ clickElement@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:214:29␊ receiveMessage@chrome://remote/content/marionette/actors/MarionetteCommandsChild.sys.mjs:97:31␊ ,
message: 'Element could not be scrolled into view',
}

› Object.throwDecodedError (node_modules/selenium-webdriver/lib/error.js:524:15)
› parseHttpResponse (node_modules/selenium-webdriver/lib/http.js:587:13)
› Executor.execute (node_modules/selenium-webdriver/lib/http.js:515:28)
› async thenableWebDriverProxy.execute (node_modules/selenium-webdriver/lib/webdriver.js:745:17)
› async test/tests/disclosure_navigation.js:76:9

combobox_grid-combo › content/patterns/combobox/examples/grid-combo.html [data-test-id="popup-key-home"]: Home from focus on list puts focus on grid and moves cursor

test/tests/combobox_grid-combo.js:880

879:
880: t.true(
881: await confirmCursorIndex(t, ex.comboboxSelector, 0),

Cursor should be at index 0 after one ARROW_HOME key

Value is not true:

false

› test/tests/combobox_grid-combo.js:880:11
› runMicrotasks ()

menu-button_actions-active-descendant › content/patterns/menu-button/examples/menu-button-actions-active-descendant.html [data-test-id="menu-end"]: "end" on role="menu"

Rejected promise returned by test. Reason:

AssertionError {
actual: 'mi3',
code: 'ERR_ASSERTION',
expected: 'mi4',
generatedMessage: false,
operator: 'strictEqual',
message: 'aria-activedescendant should be set to mi4 for item: #ex1 [role="menu"]',
}

› assertAriaSelectedAndActivedescendant (test/util/assertAriaActivedescendant.js:24:10)
› runMicrotasks ()
› async test/tests/menu-button_actions-active-descendant.js:471:3

toolbar_toolbar › content/patterns/toolbar/examples/toolbar.html [data-test-id="toolbar-button-enter-or-space"]: Test key enter on copy/paste/cut keys

test/tests/toolbar_toolbar.js:1118

1117:
1118: t.is(
1119: await buttons[copy].getAttribute('aria-disabled'),

After selecting text

Difference (- actual, + expected):

  • 'true'
  • 'false'

› test/tests/toolbar_toolbar.js:1118:7
› runMicrotasks ()

toolbar_toolbar › content/patterns/toolbar/examples/toolbar.html [data-test-id="toolbar-button-enter-or-space"]: Test key space on copy/paste/cut keys

test/tests/toolbar_toolbar.js:1209

1208:
1209: t.is(
1210: await buttons[copy].getAttribute('aria-disabled'),

After selecting text

Difference (- actual, + expected):

  • 'true'
  • 'false'

› test/tests/toolbar_toolbar.js:1209:7
› runMicrotasks ()

5 tests failed
6 known failures

For Windows + Node 16 or 18, I get the expected result when running the regression tests.

@howard-e
Copy link
Contributor Author

howard-e commented Jul 11, 2023

@mcking65 so after discussing this internally, what was found is that Node v17+ deprioritised IPv4 addresses in favour of IPv6 with a change to dns.setDefaultResultOrder. This was done without a happy-eyeballs implementation which meant the loopback address used when running the regression tests resolved to ::1 rather than the IPv4 loopback, 127.0.0.1 which geckodriver expected.

Alternatively, we could have added --dns-result-order=ipv4first to the running script, or added dns.setDefaultResultOrder('ipv4first');, but would eventually remove it as the happy-eyeballs implementation is now included in v20.

PR #2749 should resolve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Code defects; not for inaccurate prose Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants