Add legacy bundle to test old browser compatibility #233

cdata · 2018-11-22T02:13:26Z

Important details about this change:

IE11 is now supported via a "legacy" bundle and tested on CI
Splits WCT configuration into three config files due to a bug in how wct parses command line options (filed WCT ignores --local when sauce plugin is skipped Polymer/tools#798 to track)
Configurations cover the following territory in order:
1. Local Chrome (quick sanity check before running slower sauce tests)
2. Battery of "modern" browsers via sauce
3. Battery of "legacy" browsers via sauce, using "legacy" test entry point and bundle
Only Chrome is tested "locally," due to Firefox not actually running any tests in this mode on CI
- Headless Firefox does not support WebGL and it was previously failing in a flakey way just before the 4th test
Chrome 41 has been removed from the testing regime, as it was suffering the same issue as headless Firefox in CI only
- I tried every platform configuration of this browser (Windows, Mac and Linux) and it always fails to create a WebGL context. It works just fine when I run Chrome 41 locally.
polymer-build is used to generate a "legacy"-compatible bundle
- The amount of configuration to do this ourselves is rather large and complicated by the need for language feature polyfills; polymer-build is maintained and updated by a dedicated team of folks so it saves us some of that burden.
IE11 doesn't have remove on elements, so all usages have been changed to a more conservative check for parentNode and then using removeChild on that node if it exists.
We were previously misconfiguring the test timeout, this has been fixed
For a complete build, the following browsers are currently tested:
- Chrome 70 Desktop
- Chrome 70 Desktop (tested against "legacy" bundle)
- Chrome 70 Android
- Firefox 62 (pre-Web Components)
- Firefox 63 (latest / post-Web Components)
- Safari 12 Desktop
- Safari 12 iOS
- Safari 11 Desktop
- Edge 17
- Internet Explorer 11 (tested against "legacy" bundle)

Fixes #99

jsantell · 2018-11-27T22:13:06Z

.travis.yml

 script:
- xvfb-run npm run test
- if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then wct --plugin sauce; fi
+- xvfb-run npm run test && if [ "${TRAVIS_PULL_REQUEST}" = "false" ]; then ./scripts/run-sauce-tests.sh; fi


Does this only run the sauce tests when a commit lands on master?

The environment variable is a little misleading. Basically this only runs sauce tests for first-party PRs (so that malicious users cannot burn our finite sauce time) and for merges into master. TBD how we should go about running the full battery of browsers against third-party PRs, but open to suggestions.

jsantell

Great work! One thing that I'd like is some sort of info on what features trigger the need for a legacy build/test, e.g. something in the readme Browser Support section

jsantell · 2018-11-27T22:15:41Z

scripts/run-sauce-tests.sh

+#!/usr/bin/env bash
+
+set -e
+set -x


Why do you need to flip the chmod flags?

These aren't chmod flags. These set flags tell the shell to:

Exit the script on the first non-zero exit code (otherwise it would invoke all commands even if they fail)

Print all the commands being run by the script as they are invoked

See https://www.gnu.org/software/bash/manual/html_node/The-Set-Builtin.html#The-Set-Builtin

jsantell · 2018-11-27T22:17:29Z

test.html

@@ -21,11 +21,13 @@
  <meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">


Could we move test.html and test-legacy.html to test/ maybe? Or test/index.html and test/legacy.html? Could also contain a README about writing tests (in the future) -- main rational being to minimize things in the root

jsantell · 2018-11-27T22:17:49Z

test.html

@@ -45,13 +47,13 @@
  <!-- Resize Observer polyfill is required for non-Chrome browsers: -->
  <script src="./node_modules/resize-observer-polyfill/dist/ResizeObserver.js"></script>

-  <!-- Fullscreen polyfill is required to support all stable browsers: -->
+  <!-- Fullscreen polyfill is required to support Web XR-based AR in stable browsers that support it: -->


nit: s/Web XR/WebXR

jsantell · 2018-11-27T22:18:58Z

wct-legacy.conf.json

+          "version": "11.285"
+        },
+        {
+          "browserName": "safari",


Does Safari 11.1 need the legacy build?

Probably not. I'll move it up.

Now that I think about it, it's probably valuable to ensure latest Chrome works against the legacy bundle, as the code is changed very significantly.

jsantell · 2018-11-27T22:20:48Z

test-legacy.html

+
+  <!-- The following libraries and polyfills are recommended to maximize browser support -->
+  <!-- Include prismatic.js for Magic Leap support: -->
+  <!--<script src="../node_modules/@magicleap/prismatic/prismatic.min.js"></script>-->


This should probably be removed here and in test.html since it doesn't actually do anything when running tests AFAIK (we don't run tests on Helio or shim prismatic connection)

cdata · 2018-11-28T00:39:39Z

Added docs about the legacy bundle and Custom Elements ES5 Adapter

cdata force-pushed the fix-ie11 branch 10 times, most recently from 5134eda to 61adedb Compare November 27, 2018 20:05

cdata requested a review from jsantell November 27, 2018 20:41

cdata changed the title ~~WIP Add legacy bundle to test old browser compatibility~~ Add legacy bundle to test old browser compatibility Nov 27, 2018

jsantell reviewed Nov 27, 2018

View reviewed changes

jsantell previously approved these changes Nov 27, 2018

View reviewed changes

cdata dismissed jsantell’s stale review via 4f1c723 November 27, 2018 23:25

cdata force-pushed the fix-ie11 branch 4 times, most recently from a45f85e to d98b34c Compare November 28, 2018 00:12

Add legacy bundle to test old browser compatibility

4097fa0

cdata force-pushed the fix-ie11 branch from d98b34c to 4097fa0 Compare November 28, 2018 00:36

jsantell approved these changes Nov 28, 2018

View reviewed changes

cdata merged commit 72fae39 into master Nov 28, 2018

cdata deleted the fix-ie11 branch November 28, 2018 00:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add legacy bundle to test old browser compatibility #233

Add legacy bundle to test old browser compatibility #233

cdata commented Nov 22, 2018 •

edited

Loading

jsantell Nov 27, 2018

cdata Nov 27, 2018

jsantell left a comment

jsantell Nov 27, 2018

cdata Nov 27, 2018 •

edited

Loading

jsantell Nov 27, 2018

cdata Nov 27, 2018

cdata Nov 28, 2018

jsantell Nov 27, 2018

cdata Nov 27, 2018

cdata Nov 28, 2018

jsantell Nov 27, 2018

cdata Nov 27, 2018

cdata Nov 28, 2018

jsantell Nov 27, 2018

cdata Nov 27, 2018

cdata Nov 28, 2018

cdata commented Nov 28, 2018

		@@ -21,11 +21,13 @@
		<meta name="viewport" content="width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes">

Add legacy bundle to test old browser compatibility #233

Add legacy bundle to test old browser compatibility #233

Conversation

cdata commented Nov 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsantell left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdata Nov 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cdata commented Nov 28, 2018

cdata commented Nov 22, 2018 •

edited

Loading

cdata Nov 27, 2018 •

edited

Loading