-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Test IE9 by mocking window globals #1299
Conversation
Looks like a good test of the exact change I made here - thanks! 💃 That said, I'm not sure how realistic this approach really is, or could be made to be. For one, a complete test would need to mock globals before importing Plotly, as some of the errors occur when the code is first loaded rather than when it runs (which, if we had done it in this case, would have necessitated loading a subset of the full bundle). Also, simply setting a property of |
- in bundle_tests/ - mock IE9 env before loading plotly.js + test bundle
Very good point here. Thanks very much. Here's what I came up with 4ce9df0 Note that each suite under |
|
||
it('[wip] check that ie9_mock.js did its job', function() { | ||
expect(window.ArrayBuffer).toBeUndefined(); | ||
expect(window.Uint8Array).toBeUndefined(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool - can we also do something like
expect(function() { return ArrayBuffer; }).toThrow('ReferenceError: ArrayBuffer is not defined');
to make sure it's really inaccessible in the way it actually gets invoked in the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in 7cb691e
This reverts commit c0490d6.
Nice, I thought it would be harder to get that to work! 💃 |
which should help with @alexcjohnson 's #1297 and should be less annoying to setup / maintain than the
appveyor
CI.