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

Test IE9 by mocking window globals #1299

Merged
merged 4 commits into from Jan 13, 2017
Merged

Test IE9 by mocking window globals #1299

merged 4 commits into from Jan 13, 2017

Conversation

etpinard
Copy link
Contributor

which should help with @alexcjohnson 's #1297 and should be less annoying to setup / maintain than the appveyor CI.

@alexcjohnson
Copy link
Contributor

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 window to undefined is not enough, it would need to actually throw an error if you try to access it. Can you delete it instead? We could define a getter for it that throws an error, but then I'm not sure if typeof would could be made to yield undefined in case anyone looks for the property that way...

- in bundle_tests/
- mock IE9 env before loading plotly.js + test bundle
@etpinard
Copy link
Contributor Author

For one, a complete test would need to mock globals before importing Plotly

Very good point here. Thanks very much. Here's what I came up with 4ce9df0

Note that each suite under test/jasmine/bundle_tests/ is run in a different browser window via the test_bundle.js task.


it('[wip] check that ie9_mock.js did its job', function() {
expect(window.ArrayBuffer).toBeUndefined();
expect(window.Uint8Array).toBeUndefined();
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 7cb691e

@alexcjohnson
Copy link
Contributor

Nice, I thought it would be harder to get that to work! 💃

@etpinard etpinard merged commit 675103c into ie9-fix Jan 13, 2017
@etpinard etpinard deleted the test-ie9 branch January 13, 2017 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants