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

Enable Sauce runs on IE on an opt-in basis #11030

Merged
merged 20 commits into from
Aug 24, 2017
Merged

Enable Sauce runs on IE on an opt-in basis #11030

merged 20 commits into from
Aug 24, 2017

Conversation

rsimha
Copy link
Contributor

@rsimha rsimha commented Aug 22, 2017

This PR does the following:

  1. Adds IE 11 to the mix of browsers used on sauce labs for testing
  2. Disables testing on IE 11 by default
  3. Adds functions enableIe and ifIe to allow test writers to opt in to testing on IE
  4. Updates all integration tests that use just describe( with describe().configure().run(, so that the new default TestConfig is picked up

In the new world:

  • describe('foo') is no longer meant to be used for integration tests (if used, it will include IE)
  • describe().configure().run() will run tests on all browsers except IE
  • describe().configure().enableIe().run() will run tests on all browsers including IE
  • describe().configure().ifIe().run() will run tests only on IE

Fixes #10731

Copy link
Member

@cramforce cramforce left a comment

Choose a reason for hiding this comment

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

Lets opt in at least one test.

@rsimha
Copy link
Contributor Author

rsimha commented Aug 22, 2017

Opted in amp-bind tests.

@rsimha rsimha requested a review from chenshay as a code owner August 23, 2017 20:38
@rsimha
Copy link
Contributor Author

rsimha commented Aug 23, 2017

@choumx, we now have a working solution for sauce labs testing on IE. See PR description for details.

I've gone ahead and enabled a few integration tests like amp-bind on IE to see if they pass or not.

@rsimha rsimha self-assigned this Aug 23, 2017
@rsimha
Copy link
Contributor Author

rsimha commented Aug 23, 2017

@choumx, @cramforce, we now have IE 11 in our browser matrix. I've enabled a bunch of tests on IE 11: see https://travis-ci.org/ampproject/amphtml/jobs/267750202

This PR is ready for review.

@rsimha rsimha dismissed stale reviews from jridgewell and cramforce August 24, 2017 20:01

Comments have been addressed.

@rsimha
Copy link
Contributor Author

rsimha commented Aug 24, 2017

All comments are addressed, and we now have more than a dozen tests running and passing on IE 11. Merging this PR.

@rsimha rsimha merged commit ef4a661 into ampproject:master Aug 24, 2017
@rsimha rsimha deleted the 2017-08-22-IE11 branch August 24, 2017 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants