🏗🐛 Add a more robust mechanism for dynamically skipping tests (#17265)

ampproject · Aug 3, 2018 · 179dd2e · 179dd2e
1 parent df73875
commit 179dd2e
Showing 6 changed files with 526 additions and 506 deletions.
diff --git a/build-system/tasks/presubmit-checks.js b/build-system/tasks/presubmit-checks.js
@@ -612,6 +612,14 @@ const forbiddenTerms = {
       'extensions/amp-access/0.1/iframe-api/access-controller.js',
     ],
   },
+  'this\\.skip\\(\\)': {
+    message: 'Use of `this.skip()` is forbidden in test files. Use ' +
+        '`this.skipTest()` from within a `before()` block instead. See #17245.',
+    checkInTestFolder: true,
+    whitelist: [
+      'test/_init_tests.js',
+    ],
+  },
 };
 
 const ThreePTermsMessage = 'The 3p bootstrap iframe has no polyfills loaded' +

diff --git a/test/_init_tests.js b/test/_init_tests.js
@@ -52,10 +52,6 @@ const originalConsoleError = console/*OK*/.error;
 let initialGlobalState;
 let initialWindowState;
 
-// TODO(rsimha, #17245): Temporary. Remove this. Needed due to
-// https://github.com/mochajs/mocha/issues/2148.
-let sandboxCreated;
-
 // All exposed describes.
 global.describes = describes;
 
@@ -390,17 +386,22 @@ function preventAsyncErrorThrows() {
   stubAsyncErrorThrows();
 }
 
+before(function() {
+  // This is a more robust version of `this.skip()`. See #17245.
+  this.skipTest = function() {
+    if (this._runnable.title != '"before all" hook') {
+      throw new Error('skipTest() can only be called from within before()');
+    }
+    this.test.parent.pending = true; // Workaround for mochajs/mocha#2683.
+    this.skip();
+  };
+});
+
 beforeEach(function() {
   this.timeout(BEFORE_AFTER_TIMEOUT);
   beforeTest();
   testName = this.currentTest.fullTitle();
-  if (sandboxCreated) {
-    sinon.sandbox.restore();
-    consoleErrorSandbox.restore();
-    rethrowAsyncSandbox.restore();
-  }
   sinon.sandbox = sinon.createSandbox();
-  sandboxCreated = true;
   maybeStubConsoleInfoLogWarn();
   preventAsyncErrorThrows();
   warnForConsoleError();
@@ -430,7 +431,6 @@ afterEach(function() {
   const globalState = Object.keys(global);
   const windowState = Object.keys(window);
   sinon.sandbox.restore();
-  sandboxCreated = false;
   restoreConsoleError();
   restoreAsyncErrorThrows();
   this.timeout(BEFORE_AFTER_TIMEOUT);

diff --git a/test/functional/test-fixed-layer.js b/test/functional/test-fixed-layer.js
@@ -1321,7 +1321,7 @@ describes.sandboxed('FixedLayer', {}, () => {
 });
 
 
-describes.realWin('FixedLayer: shadow transfer', {}, env => {
+describes.realWin('FixedLayer', {}, env => {
   let win, doc;
   let ampdoc;
   let fixedLayer;
@@ -1332,83 +1332,83 @@ describes.realWin('FixedLayer: shadow transfer', {}, env => {
   let shadowRoot;
   let container;
 
-  beforeEach(function() {
-    if (!env.win.Element.prototype.attachShadow) {
-      // Can only test when SD is supported.
-      this.skip();
-    }
-    win = env.win;
-    doc = win.document;
-    vsyncTasks = [];
-    vsyncApi = {
-      runPromise: task => {
-        vsyncTasks.push(task);
-        return Promise.resolve();
-      },
-      mutate: mutator => {
-        vsyncTasks.push({mutate: mutator});
-      },
-    };
-    installPlatformService(win);
-    ampdoc = new AmpDocSingle(win);
-    shadowRoot = win.document.body.attachShadow({mode: 'open'});
-    fixedLayer = new FixedLayer(ampdoc, vsyncApi,
-        /* borderTop */ 0, /* paddingTop */ 11, /* transfer */ true);
-    fixedLayer.setup();
-    transferLayer = fixedLayer.getTransferLayer_();
-    root = transferLayer.getRoot();
-    container = doc.createElement('div');
-    doc.body.appendChild(container);
-  });
+  // Can only test when Shadow DOM is available.
+  describe.configure().if(() => Element.prototype.attachShadow).run('shadow ' +
+      'transfer', function() {
+    beforeEach(function() {
+      win = env.win;
+      doc = win.document;
+      vsyncTasks = [];
+      vsyncApi = {
+        runPromise: task => {
+          vsyncTasks.push(task);
+          return Promise.resolve();
+        },
+        mutate: mutator => {
+          vsyncTasks.push({mutate: mutator});
+        },
+      };
+      installPlatformService(win);
+      ampdoc = new AmpDocSingle(win);
+      shadowRoot = win.document.body.attachShadow({mode: 'open'});
+      fixedLayer = new FixedLayer(ampdoc, vsyncApi,
+          /* borderTop */ 0, /* paddingTop */ 11, /* transfer */ true);
+      fixedLayer.setup();
+      transferLayer = fixedLayer.getTransferLayer_();
+      root = transferLayer.getRoot();
+      container = doc.createElement('div');
+      doc.body.appendChild(container);
+    });
 
-  it('should create layer correctly', () => {
-    expect(root.parentNode).to.equal(shadowRoot);
-    expect(root.id).to.equal('i-amphtml-fixed-layer');
-    expect(root.style.position).to.equal('absolute');
-    expect(root.style.top).to.equal('0px');
-    expect(root.style.left).to.equal('0px');
-    expect(root.style.width).to.equal('0px');
-    expect(root.style.height).to.equal('0px');
-    expect(root.style.overflow).to.equal('hidden');
-    expect(root.style.visibility).to.equal('');
-    expect(root.children).to.have.length(1);
-    expect(root.children[0].tagName).to.equal('SLOT');
-    expect(root.children[0].name).to.equal('i-amphtml-fixed');
-  });
+    it('should create layer correctly', () => {
+      expect(root.parentNode).to.equal(shadowRoot);
+      expect(root.id).to.equal('i-amphtml-fixed-layer');
+      expect(root.style.position).to.equal('absolute');
+      expect(root.style.top).to.equal('0px');
+      expect(root.style.left).to.equal('0px');
+      expect(root.style.width).to.equal('0px');
+      expect(root.style.height).to.equal('0px');
+      expect(root.style.overflow).to.equal('hidden');
+      expect(root.style.visibility).to.equal('');
+      expect(root.children).to.have.length(1);
+      expect(root.children[0].tagName).to.equal('SLOT');
+      expect(root.children[0].name).to.equal('i-amphtml-fixed');
+    });
 
-  it('should transfer element', () => {
-    const element = doc.createElement('div');
-    container.appendChild(element);
-    const fe = {element, id: 'F0'};
-    transferLayer.transferTo(fe);
-
-    // Element stays where it was.
-    expect(element.parentElement).to.equal(container);
-    expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
-    expect(root.children).to.have.length(1);
-
-    // Ensure that repeat slotting doesn't change anything.
-    transferLayer.transferTo(fe);
-    expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
-    expect(root.children).to.have.length(1);
-  });
+    it('should transfer element', () => {
+      const element = doc.createElement('div');
+      container.appendChild(element);
+      const fe = {element, id: 'F0'};
+      transferLayer.transferTo(fe);
+
+      // Element stays where it was.
+      expect(element.parentElement).to.equal(container);
+      expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
+      expect(root.children).to.have.length(1);
+
+      // Ensure that repeat slotting doesn't change anything.
+      transferLayer.transferTo(fe);
+      expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
+      expect(root.children).to.have.length(1);
+    });
 
-  it('should return element', () => {
-    const element = doc.createElement('div');
-    container.appendChild(element);
-    const fe = {element, id: 'F0'};
-    transferLayer.transferTo(fe);
-    expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
-    expect(root.children).to.have.length(1);
-
-    // The slot distribution is canceled, but the slot itself is kept.
-    transferLayer.returnFrom(fe);
-    expect(element.getAttribute('slot')).to.be.null;
-    expect(root.children).to.have.length(1);
-
-    // Ensure that repeat slotting doesn't change anything.
-    transferLayer.transferTo(fe);
-    expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
-    expect(root.children).to.have.length(1);
+    it('should return element', () => {
+      const element = doc.createElement('div');
+      container.appendChild(element);
+      const fe = {element, id: 'F0'};
+      transferLayer.transferTo(fe);
+      expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
+      expect(root.children).to.have.length(1);
+
+      // The slot distribution is canceled, but the slot itself is kept.
+      transferLayer.returnFrom(fe);
+      expect(element.getAttribute('slot')).to.be.null;
+      expect(root.children).to.have.length(1);
+
+      // Ensure that repeat slotting doesn't change anything.
+      transferLayer.transferTo(fe);
+      expect(element.getAttribute('slot')).to.equal('i-amphtml-fixed');
+      expect(root.children).to.have.length(1);
+    });
   });
 });