search

w3c / aria-practices

WAI-ARIA Authoring Practices Guide (APG)
https://www.w3.org/wai/aria/apg/
Other
1.21k stars 339 forks source link

Regression tests fail on macOS due to Windows-based assumptions #2735

Open mcking65 opened 1 year ago

mcking65 commented 1 year ago

@frozenzia wrote:

In toolbar_toolbar.js, there are a couple of tests where CTRL+A are currently used to select all the text in a textarea. This does nothing on a Mac, and copying similar code from alertdialog_alertdialog.js, I've added an extra line to do the selecting in case the test is run on a Mac. After making this one and only change, running the tests still results in 2 test failures (+ 6 known failures):

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

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)
› async test/tests/menu-button_actions-active-descendant.js:471:3

─

2 tests failed
6 known failures

If I DON'T add those 2 lines in the toolbar test, I actually end up with 5 tests failed:

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

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)
› async test/tests/menu-button_actions-active-descendant.js:471:3

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 <a href="#mythical-page-content"> 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

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:1119

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

After selecting text

Difference (- actual, + expected):

- 'true'
+ 'false'

› test/tests/toolbar_toolbar.js:1119:7

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:1211

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

After selecting text

Difference (- actual, + expected):

- 'true'
+ 'false'

› test/tests/toolbar_toolbar.js:1211:7

─

5 tests failed
6 known failures

Originally posted by @frozenzia in https://github.com/w3c/aria-practices/issues/2628#issuecomment-1601967708

frozenzia commented 1 year ago

@mcking65, should this be even 2 issues? One the issue of the tests failing b/c of Windows-based assumptions, but the other of the remaining 2 tests that fail on macOS for ... 🤷🏽‍♀️.

css-meeting-bot commented 1 year ago

The ARIA Authoring Practices (APG) Task Force just discussed Running tests on macOS.

The full IRC log of that discussion <jugglinmike> Topic: Running tests on macOS
<jugglinmike> Andrea_Cardona: Running this on my end, I can see a number of files checked and a timeout reported by Ava. It tells me that 57 hooks failed
<jugglinmike> Matt_King: There are exactly 57 examples, so that's all of them
<jugglinmike> Andrea_Cardona: I believe the errors I'm observing are different than what the reporter has shared
<jugglinmike> github: https://github.com/w3c/aria-practices/issues/2735
<jugglinmike> Matt_King: Paul was following the instructions for new contributors and ran into this problem right here. It's quite distracting, and I'm very concerned that we have instructions for new contributors that are leading them astray
<jugglinmike> Matt_King: We designed the regression tests to be able to run them locally, and I think they SHOULD be run locally, but if this is going to take a long time to address, I wonder if we should change the recommendation to rely on the CI system to run the regression tests
<jugglinmike> Andrea_Cardona: Yeah, arigilmore and I have relied on the CI system in that way in the past
<jugglinmike> Andrea_Cardona: It's somewhat slow, though!
<jugglinmike> jugglinmike: It can also be intimidating for new contributors to make their first pull request. It would be intimidating to have to wait for this automated feedback until *after* you've shared your work publicly
<jugglinmike> howard-e: They run reliably for me when I use Node.js 16 instead of Node.js 18. I'd be reluctant to pin to an old release of Node.js, though
<jugglinmike> Zakim, end the meeting
evmiguel commented 7 months ago

@frozenzia Before I can move on with this task, I just want to align on your Mac environment for running the regression tests. In test/index.js I had to add the following to be able to run the regression tests:

...
const firefox = require('selenium-webdriver/firefox');
...

let options = new firefox.Options();
options.addArguments('-headless');
options.setBinary('/Applications/Firefox.app/Contents/MacOS/firefox');

If you have any workarounds to this, please let me know. I also wanted to align that we are seeing the same regression results. My regression output without the proposed changes are as follows - not sure if they have changed:

  9 tests failed
  6 known failures
  29 tests remained pending after a timeout

I can confirm the 6 known failures. As for the other results, I'd like to make sure that we match. From my understanding, there were 5 failing tests upon the last comment in June, but now, there are 9 failing tests.

frozenzia commented 7 months ago

@evmiguel Sorry for the delayed response - I'll try to get to this in the next day or two and get back to you.

frozenzia commented 6 months ago

@evmiguel, alright, sorry, finally got to this. Using main branch, which had as its last commit 81d807dfa6dc0eea7f455dfd540af0c363845274 (from Feb 26, 2024). I have not had to touch test/index.js to get this working. I still get 5 tests failed, 6 known failures. However, with the following difference:

In June the failing cases were in:

Now the failing cases are in:

AND, again if I make the same change to toolbar_toolbar.js, specifically at lines 1119 and 1211:

   async (t) => {
     let textarea = await t.context.session.findElement(By.css('textarea'));
     await textarea.sendKeys(Key.chord(Key.CONTROL, 'a'));
+    await textarea.sendKeys(Key.chord(Key.COMMAND, 'a')); //macOS
     let originalText = await textarea.getAttribute('value');

   async (t) => {
     let textarea = await t.context.session.findElement(By.css('textarea'));
     await textarea.sendKeys(Key.chord(Key.CONTROL, 'a'));
+    await textarea.sendKeys(Key.chord(Key.COMMAND, 'a')); //macOS
     let originalText = await textarea.getAttribute('value');

With that change there are only 2 tests failed, 6 known failures. The 2 failed tests are in:

evmiguel commented 5 months ago

@frozenzia Thanks for getting back to me. I like your idea of detecting OS for managing edit fields. Are there any other tests you found that require this other than toolbar_toolbar.js?