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

Space key and Selenium testing framework #1358

Closed
jongund opened this issue Mar 24, 2020 · 3 comments
Closed

Space key and Selenium testing framework #1358

jongund opened this issue Mar 24, 2020 · 3 comments
Assignees
Labels
Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation question Issue asking a question regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo

Comments

@jongund
Copy link
Contributor

jongund commented Mar 24, 2020

In the updated version of the Javascript for menubar-editor I switched from using event.keyCode to event.key property. The testing script menbar_menubar-navigator.js (line 500) has a test for the SPACE key and it was failing even though it worked with manual testing on the example. So I added a couple lines of code in menubar-editor.js to detect event.keyCode === 32 and if this is true change event.key to equal (see line 524). The test script now passed for all tests including the SPACE key test. Seems to me a bug with Selenium, but maybe it is something I am doing.

Test Files with "failing" tests due to the SPACE key issue:

  • /test/tests/combobox_datepicker.js
  • /test/tests/dialog-modal-datepicker.js
  • /test/tests/menubar_menubar-navigation.js
  • /test/tests/menubar_menubar-editor.js
@mcking65 mcking65 added Infrastructure Related to maintaining task force and repo operations, processes, systems, documentation question Issue asking a question regression-testing Related to AVA regression tests of example pages or AVA framework implementation within repo labels Mar 31, 2020
@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Space key and Selenium testing framework.

The full IRC log of that discussion <MarkMccarthy> TOPIC: Space key and Selenium testing framework
<MarkMccarthy> jongund: everything's fine in the browser, but the code is funky. it's affecting everything i've done in the last couple weeks
<MarkMccarthy> mck: 1350 is kind of the root of this, right?
<MarkMccarthy> github: https://github.com//issues/1358
<Jemma> KeyboardEvent.keyCode Deprecated #1350
<MarkMccarthy> jongund: that's not a problem, we can move to <key>. the problem seems to be selenium
<MarkMccarthy> Jemma: maybe valerie could help us, since she helped with the framework
<MarkMccarthy> mck: i put 1358 in simon's queue for infrastructure
<MarkMccarthy> mck: whats unclear is what the solution is.
<MarkMccarthy> mck: was also wondering - some things in 1350...would it make sense to have a utility class or utils script to define some of this? so it's only in one place, then reuse from there?
<Jemma> s/the framework/the regression testing framework
<MarkMccarthy> jongund: i don't think we really need a utility. the main issue with <key> is that edge used different constants
<MarkMccarthy> jongund: and now that they're moving to chrome, things might be different. i don't think a utility would help. all the new event handlers are a lot more streamlines
<MarkMccarthy> s/streamlines/streamlined
<MarkMccarthy> jongund: and everything is a bit more literal and eaiser to read
<MarkMccarthy> jongund: the previous examples just used numerical constants, so this is easier. but the space issue is frustrating, especially becuase it's -just- the regression tests that are having an issue.
<MarkMccarthy> jongund: maybe we just let those tests fail for now, and rewrite the code correctly.
<ZoeBijl> regrets+
<MarkMccarthy> mck: so right now, this affects 4 PRs, right?
<MarkMccarthy> mck: so if we let them fail and allow these things to be merge-able, we can leave 1358 open and for each example marked as failing, add it as a checklist in 1358
<MarkMccarthy> jongund: that might work
<MarkMccarthy> mck: i'd be alright with that, to help us keep track
<MarkMccarthy> jongund: and to keep track of new issues as they arise
<MarkMccarthy> mck: yeah. but then we can keep track and whoever takes up 1358 will have a list to work off of
<MarkMccarthy> mck: or maybe a new version of Selenium will fix it who knows
<MarkMccarthy> jongund: right, maybe it's just an obscure bug
<MarkMccarthy> mck: okay, so let's use that plan for now.

@smhigley
Copy link
Contributor

smhigley commented Apr 1, 2020

It appears there are known issues with GeckoDriver + Selenium + sendKeys(Key.SPACE) on an element that is not a text input. This works if you use sendKeys(' ') instead, or the Actions API.

Refs:
mozilla/geckodriver#777
mozilla/geckodriver#846
https://bugzilla.mozilla.org/show_bug.cgi?id=1068733

I'm going to close this since there are multiple workarounds, though anyone can reopen if they feel there's more discussion needed.

@jongund
Copy link
Contributor Author

jongund commented Apr 1, 2020

@smhigley

Sara thank you for figuring it out!

Jon

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

No branches or pull requests

5 participants