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

chore(focus-classes): fix failing tests when browser is blurred #2935

Merged

Conversation

devversion
Copy link
Member

@devversion devversion commented Feb 4, 2017

On Saucelabs, browsers will run simultaneously and therefore can't focus all browser windows at the same time.

This is problematic when testing focus states. Chrome and Firefoxonly fire FocusEvents when the window is focused. This issue also appears locally.

Also while investigating the karma-test-shim.js file looked very ugly, so I refactored it a bit (ES6 not allowed)

Fixes #2903.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Feb 4, 2017
@devversion devversion force-pushed the fix/focus-classes-firefox-tests branch from 8014fe9 to 09d14cf Compare February 4, 2017 21:23
@devversion devversion changed the title fix(focus-classes): tests for firefox on saucelabs chore(focus-classes): fix failing tests when browser is blurred Feb 4, 2017
@devversion devversion force-pushed the fix/focus-classes-firefox-tests branch from 09d14cf to 6deb9db Compare February 4, 2017 21:24
On Saucelabs, browsers will run simultaneously and therefore can't focus all browser windows at the same time.
This is problematic when testing focus states. Chrome and Firefoxonly fire FocusEvents when the window is focused. This issue also appears locally.

Fixes angular#2903.
@mmalerba
Copy link
Contributor

mmalerba commented Feb 5, 2017

Are we not able to set the focusmanager.testmode pref? Are you sure this is a problem in chrome too? My tests seem to consistently pass in chrome

@devversion
Copy link
Member Author

Not really. Chrome does not have such a flag and setting it for Firefox in Saucelabs doesn't seem to be possible.

Kristiyan and I have the same problems locally (when not focused). Also IE11 seems to have similar problems on Saucelabs.

I think that this approach is be the most reliable one.

// On Saucelabs, browsers will run simultaneously and therefore can't focus all browser windows
// at the same time. This is problematic when testing focus states. Chrome and Firefox
// only fire FocusEvents when the window is focused. This issue also appears locally.
let _nativeButtonFocus = buttonElement.focus.bind(buttonElement);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you extract this into a separate method e.g. fixFocus(el: Element)? also both this and dispatchFocusEvent would be good things to put in a separate test utilities directory

Copy link
Member Author

@devversion devversion Feb 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. I called it patchElementFocus - also added a comment, so we can move all helpers to a global utility file as in #2902

@crisbeto
Copy link
Member

crisbeto commented Feb 5, 2017

What version of Chrome are you running @mmalerba? I think 56 started aggresively throttling background tabs, which might be what's causing the issue here.

@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Feb 6, 2017
@tinayuangao tinayuangao merged commit 478a2e4 into angular:master Feb 8, 2017
@devversion devversion deleted the fix/focus-classes-firefox-tests branch February 8, 2017 23:03
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(tests): firefox doesn't fire focus/blur events in tests
5 participants