[amp-sidebar] re-enable flaky visual tests (#27495)

* Wait for element to build * Disable animations on test pages * Re-enable sidebar opening test * Don't disable animations
ampproject · Mar 31, 2020 · 161cab0 · 161cab0
1 parent 6c93cd9
commit 161cab0
Show file tree

Hide file tree

Showing 6 changed files with 44 additions and 23 deletions.
diff --git a/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ol.amp.html b/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ol.amp.html
@@ -32,9 +32,6 @@
       margin: 0;
     }
 
-    amp-sidebar ul {
-      list-style: none;
-    }
     amp-sidebar li {
       cursor: pointer;
       height: 30px;
@@ -44,6 +41,12 @@
       width: 50vw;
     }
 
+    /* Using invalid AMP to target the sidebar. 
+    Disable transforms for visual diff test */
+    .i-amphtml-sidebar-mask, amp-sidebar[side] {
+      transform: none;
+    }
+
     #mybody section{
       background: red;
     }
@@ -68,7 +71,7 @@
 
   <h1>AMP Sidebar Example</h1>
   <button on='tap:sidebar.toggle'
-    aria-label="Click to toglge sidebar">
+    aria-label="Click to toggle sidebar">
     Toggle sidebar
   </button>
   <div id="target-id"></div>
@@ -97,5 +100,7 @@ <h1>AMP Sidebar Example</h1>
       </div>
     </div>
   </article>
+  <div id="left-target-id">
+  </div>
 </body>
 </html>
diff --git a/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ul.amp.html b/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ul.amp.html
@@ -44,6 +44,12 @@
       width: 50vw;
     }
 
+    /* Using invalid AMP to target the sidebar. 
+    Disable transforms for visual diff test */
+    .i-amphtml-sidebar-mask, amp-sidebar[side] {
+      transform: none;
+    }
+
     #mybody section{
       background: red;
     }
@@ -68,7 +74,7 @@
 
   <h1>AMP Sidebar Example</h1>
   <button on='tap:sidebar.toggle'
-    aria-label="Click to toglge sidebar">
+    aria-label="Click to toggle sidebar">
     Toggle sidebar
   </button>
   <div id="target-id"></div>
@@ -97,5 +103,8 @@ <h1>AMP Sidebar Example</h1>
       </div>
     </div>
   </article>
+
+  <div id="left-target-id">
+  </div>
 </body>
 </html>
diff --git a/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar.js b/examples/visual-tests/amp-sidebar/amp-sidebar-toolbar.js
@@ -18,14 +18,14 @@
  */
 'use strict';
 
-const {verifySelectorsVisible} = require('../../../build-system/tasks/visual-diff/helpers');
+const {
+  verifySelectorsVisible,
+} = require('../../../build-system/tasks/visual-diff/helpers');
 
 module.exports = {
-  // Since the sidebar now uses animations to open/close, the screenshots will
-  // not capture the correct state (Percy does not run animations).
-  // 'open sidebar toolbar': async (page, name) => {
-  //   await page.tap('[on="tap:sidebar.toggle"]');
-  //   await verifySelectorsVisible(page, name, ['amp-sidebar[open]']);
-  //   await verifySelectorsVisible(page, name, ['nav[toolbar]']);
-  // },
+  'open sidebar toolbar': async (page, name) => {
+    await page.tap('[on="tap:sidebar.toggle"]');
+    await verifySelectorsVisible(page, name, ['amp-sidebar[open]']);
+    await verifySelectorsVisible(page, name, ['nav[toolbar]']);
+  },
 };
diff --git a/examples/visual-tests/amp-sidebar/amp-sidebar.amp.html b/examples/visual-tests/amp-sidebar/amp-sidebar.amp.html
@@ -65,6 +65,12 @@
       width: 50vw;
     }
 
+    /* Using invalid AMP to target the sidebar. 
+    Disable transforms for visual diff test */
+    .i-amphtml-sidebar-mask, amp-sidebar[side] {
+      transform: none;
+    }
+
     #mybody section{
       background: red;
     }

diff --git a/examples/visual-tests/amp-sidebar/amp-sidebar.js b/examples/visual-tests/amp-sidebar/amp-sidebar.js
@@ -15,11 +15,13 @@
  */
 'use strict';
 
+const {
+  verifySelectorsVisible,
+} = require('../../../build-system/tasks/visual-diff/helpers');
+
 module.exports = {
-  // Since the sidebar now uses animations to open/close, the screenshots will
-  // not capture the correct state (Percy does not run animations).
-  // 'open sidebar': async (page, name) => {
-  //   await page.tap('[on="tap:sidebar1.toggle"]');
-  //   await verifySelectorsVisible(page, name, ['amp-sidebar[open]']);
-  // },
+  'open sidebar': async (page, name) => {
+    await page.tap('[on="tap:sidebar1.toggle"]');
+    await verifySelectorsVisible(page, name, ['amp-sidebar[open]']);
+  },
 };
diff --git a/test/visual-diff/visual-tests b/test/visual-diff/visual-tests
@@ -210,26 +210,25 @@
       "loading_complete_selectors": ["video.i-amphtml-replaced-content"]
     },
     {
-      // TODO(#27456, @ampproject/wg-ui-and-a11y): see https://percy.io/ampproject/amphtml/builds/4656426
-      "flaky": true,
       "url": "examples/visual-tests/amp-sidebar/amp-sidebar.amp.html",
       "name": "amp-sidebar",
       "viewport": {"width": 400, "height": 800},
       "interactive_tests": "examples/visual-tests/amp-sidebar/amp-sidebar.js",
+      "loading_incomplete_selectors": ["amp-sidebar.i-amphtml-element"]
     },
     {
       "url": "examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ol.amp.html",
       "name": "amp-sidebar toolbar > ol",
       "viewport": {"width": 400, "height": 800},
       "interactive_tests": "examples/visual-tests/amp-sidebar/amp-sidebar-toolbar.js",
+      "loading_incomplete_selectors": ["amp-sidebar.i-amphtml-element"]
     },
     {
-      // TODO(#27456, @ampproject/wg-ui-and-a11y): see https://percy.io/ampproject/amphtml/builds/4637948
-      "flaky": true,
       "url": "examples/visual-tests/amp-sidebar/amp-sidebar-toolbar-ul.amp.html",
       "name": "amp-sidebar toolbar > ul",
       "viewport": {"width": 400, "height": 800},
       "interactive_tests": "examples/visual-tests/amp-sidebar/amp-sidebar-toolbar.js",
+      "loading_incomplete_selectors": ["amp-sidebar.i-amphtml-element"]
     },
     {
       // TODO(#27455, @ampproject/wg-ads): see https://percy.io/ampproject/amphtml/builds/4672752