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

Remove Karma flakiness by providing browser shims #1119

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lelandrichardson
Copy link
Collaborator

This fixes the travis karma flakiness, and removes the allowed failure. This does this by adding in airbnb-browser-shims to the karma tests so that we don't have to worry about which version of chrome they are running on.

* to tell karma to run a file before everything else. This is the next best
* thing I guess...
*/
const isBrowser = typeof window !== 'undefined' &&
Copy link
Member

Choose a reason for hiding this comment

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

Why is a typeof window check not sufficient?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

because we screw that up w/ jsdom.

Object.prototype.toString.call(window) === '[object Window]';

if (isBrowser) {
require('airbnb-browser-shims');
Copy link
Member

Choose a reason for hiding this comment

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

This is actually both safe to require in node, and recommended to do so.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hmm. doing so caused many issues. perhaps it's related to jsdom?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yeah true, that makes sense.

@lelandrichardson
Copy link
Collaborator Author

This appears to have not actually fixed the flakiness.... so it seems like this is not the reason for the karma flakiness :/

@nfcampos
Copy link
Collaborator

nfcampos commented Sep 26, 2017

@lelandrichardson it did fix flakiness caused by lack of polyfills,it just didn't fix the other cause of flakiness,some tests appear to cause a full page reload. This should still be merged

@lelandrichardson
Copy link
Collaborator Author

@nfcampos any thoughts on how to fix the page reload issue? It'd be nice to fix karma in a single PR if possible.

@ljharb ljharb force-pushed the lmr--fix-karma-flake branch from 212f345 to 11d153e Compare August 7, 2018 07:36
@ljharb
Copy link
Member

ljharb commented Aug 7, 2018

Rebased this on top of #1096.

@ljharb ljharb force-pushed the master branch 3 times, most recently from 40cc703 to 0a17404 Compare April 12, 2019 23:05
@ljharb ljharb force-pushed the master branch 4 times, most recently from 2227326 to 0d5ead7 Compare December 21, 2020 07:46
@ljharb ljharb force-pushed the master branch 3 times, most recently from 43eb75e to 39e6b1f Compare November 3, 2022 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants