add --esm and --minified options for visual-diff task (#37115)

* add --esm and --minified options for visual-diff * switch these tests manually to mjs * apply recs * add feature to be able to skip transformations * fix flag comment
ampproject · Dec 6, 2021 · 8814a3f · 8814a3f
1 parent 66b45f4
commit 8814a3f
Show file tree

Hide file tree

Showing 10 changed files with 14 additions and 14 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -542,6 +542,7 @@ workflows:
           name: 'Visual Diff Tests'
           <<: *push_and_pr_builds
           requires:
+            - 'Module Build (Test)'
             - 'Nomodule Build (Test)'
       - local_unit_tests:
           name: 'Local Unit Tests'

diff --git a/build-system/pr-check/visual-diff-tests.js b/build-system/pr-check/visual-diff-tests.js
@@ -22,9 +22,9 @@ function pushBuildWorkflow() {
  */
 function prBuildWorkflow() {
   if (buildTargetsInclude(Targets.RUNTIME, Targets.VISUAL_DIFF)) {
-    timedExecOrDie('amp visual-diff');
+    timedExecOrDie('amp visual-diff --esm --minified');
   } else {
-    timedExecOrDie('amp visual-diff --empty');
+    timedExecOrDie('amp visual-diff --empty --esm --minified');
     skipDependentJobs(
       jobName,
       'this PR does not affect the runtime or visual diff tests'

diff --git a/build-system/server/app.js b/build-system/server/app.js
@@ -73,10 +73,7 @@ app.use(bodyParser.text());
 // Middleware is executed in order, so this must be at the top.
 // TODO(#24333): Migrate all server URL handlers to new-server/router and
 // deprecate app.js.
-// TODO(erwinmombay, #32865): Make visual diff tests use the new server
-if (!argv._.includes('visual-diff')) {
-  app.use(require('./new-server/router'));
-}
+app.use(require('./new-server/router'));
 
 app.use(require('./routes/a4a-envelopes'));
 app.use('/amp4test', require('./amp4test').app);

diff --git a/build-system/tasks/visual-diff/index.js b/build-system/tasks/visual-diff/index.js
@@ -702,12 +702,14 @@ visualDiff.description = 'Run the AMP visual diff tests';
 visualDiff.flags = {
   'main': 'Include a blank snapshot (baseline for skipped builds)',
   'empty': 'Create a dummy Percy build with only a blank snapshot',
+  'esm': 'uses the typescript server transforms which will serve the mjs files',
   'chrome_debug': 'Print debug info from Chrome',
   'webserver_debug': 'Print debug info from the local amp webserver',
   'percy_agent_debug': 'Print debug info from the @percy/agent instance',
   'debug': 'Set all debugging flags',
   'verbose': 'Print verbose log statements',
   'grep': 'Run tests that match the pattern',
+  'minified': 'Serve minified JS',
   'percy_token': 'Override the PERCY_TOKEN environment variable',
   'percy_branch': 'Override the PERCY_BRANCH environment variable',
   'percy_disabled':

diff --git a/examples/visual-tests/amp-story-player/back-button.html b/examples/visual-tests/amp-story-player/back-button.html
@@ -5,7 +5,7 @@
       // TODO(#17108): Remove this when getMode().test is set.
       __AMP_TEST = true;
     </script>
-    <script src="../../../dist/amp-story-player-v0.js"></script>
+    <script type="module" src="../../../dist/amp-story-player-v0.mjs"></script>
     <link href="../../../build/css/amp-story-player-v0.css" rel='stylesheet' type='text/css'>
     <style page>
       body {

diff --git a/examples/visual-tests/amp-story-player/close-button.html b/examples/visual-tests/amp-story-player/close-button.html
@@ -5,7 +5,7 @@
       // TODO(#17108): Remove this when getMode().test is set.
       __AMP_TEST = true;
     </script>
-    <script src="../../../dist/amp-story-player-v0.js"></script>
+    <script type="module" src="../../../dist/amp-story-player-v0.mjs"></script>
     <link href="../../../build/css/amp-story-player-v0.css" rel='stylesheet' type='text/css'>
     <style page>
       body {

diff --git a/examples/visual-tests/amp-story-player/player-amp-version.html b/examples/visual-tests/amp-story-player/player-amp-version.html
@@ -3,9 +3,9 @@
   <head>
     <meta charset="utf-8">
     <title>Story Player (AMP version)</title>
-    <script async src="https://cdn.ampproject.org/v0.js"></script>
+    <script async src="https://cdn.ampproject.org/v0.mjs"></script>
     <link rel="canonical" href="player-amp.html">
-    <script async custom-element="amp-story-player" src="https://cdn.ampproject.org/v0/amp-story-player-0.1.js"></script>
+    <script type="module" async custom-element="amp-story-player" src="https://cdn.ampproject.org/v0/amp-story-player-0.1.mjs"></script>
     <style amp-boilerplate>body{-webkit-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-moz-animation:-amp-start 8s steps(1,end) 0s 1 normal both;-ms-animation:-amp-start 8s steps(1,end) 0s 1 normal both;animation:-amp-start 8s steps(1,end) 0s 1 normal both}@-webkit-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-moz-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-ms-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@-o-keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}@keyframes -amp-start{from{visibility:hidden}to{visibility:visible}}</style><noscript><style amp-boilerplate>body{-webkit-animation:none;-moz-animation:none;-ms-animation:none;animation:none}</style></noscript>
     <meta name="viewport" content="width=device-width,minimum-scale=1,initial-scale=1">
     <script>

diff --git a/examples/visual-tests/amp-story-player/player-local-story.html b/examples/visual-tests/amp-story-player/player-local-story.html
@@ -6,7 +6,7 @@
       // TODO(#17108): Remove this when getMode().test is set.
       __AMP_TEST = true;
     </script>
-    <script async src="../../../dist/amp-story-player-v0.js"></script>
+    <script type="module" async src="../../../dist/amp-story-player-v0.mjs"></script>
     <link href="../../../build/css/amp-story-player-v0.css" rel='stylesheet' type='text/css'>
     <style page>
       body {

diff --git a/examples/visual-tests/amp-story-player/story-attribution.html b/examples/visual-tests/amp-story-player/story-attribution.html
@@ -6,7 +6,7 @@
       // TODO(#17108): Remove this when getMode().test is set.
       __AMP_TEST = true;
     </script>
-    <script async src="../../../dist/amp-story-player-v0.js"></script>
+    <script type="module" async src="../../../dist/amp-story-player-v0.mjs"></script>
     <link href="../../../build/css/amp-story-player-v0.css" rel='stylesheet' type='text/css'>
     <style page>
       body {

diff --git a/test/visual-diff/visual-tests.jsonc b/test/visual-diff/visual-tests.jsonc
@@ -351,7 +351,7 @@
       "interactive_tests": "examples/visual-tests/amp-story/amp-story-consent.js"
     },
     {
-      "url": "examples/visual-tests/amp-story/amp-story-unsupported-browser-layer.html",
+      "url": "examples/visual-tests/amp-story/amp-story-unsupported-browser-layer.html?transform=0",
       "name": "amp-story: unsupported browser",
       "viewport": {"width": 320, "height": 480},
       "loading_complete_selectors": [
@@ -428,7 +428,7 @@
       "interactive_tests": "examples/visual-tests/amp-story/amp-story-consent.js"
     },
     {
-      "url": "examples/visual-tests/amp-story/amp-story-unsupported-browser-layer.html",
+      "url": "examples/visual-tests/amp-story/amp-story-unsupported-browser-layer.html?transform=0",
       "name": "amp-story: unsupported browser (desktop)",
       "viewport": {"width": 1440, "height": 900},
       "loading_complete_selectors": [