🏗 Improve usability of describes by making it configurable

.eslintrc.js

@@ -307,12 +307,7 @@ module.exports = {
       },
     },
     {
-      'files': [
-        '**/test-*',
-        '**/_init_tests.js',
-        '**/*_test.js',
-        '**/testing/**',
-      ],
+      'files': ['**/test-*', '**/*_test.js', '**/testing/**'],
       'rules': {
         'local/no-forbidden-terms': [2, forbiddenTermsGlobal],
       },

build-system/eslint-rules/always-call-chai-methods.js

@@ -20,7 +20,6 @@
 const chai = require('chai');
 chai.use(require('chai-as-promised'));
 chai.use(require('sinon-chai'));
-// test/_init_tests.js
 chai.Assertion.addMethod('attribute');
 chai.Assertion.addMethod('class');
 chai.Assertion.addMethod('display');

build-system/tasks/runtime-test/runtime-test-base.js

@@ -331,7 +331,7 @@ class RuntimeTestConfig {
       adTypes: getAdTypes(),
       mochaTimeout: this.client.mocha.timeout,
       testServerPort: this.client.testServerPort,
-      isModuleBuild: !!argv.esm, // Used by skip matchers in _init_tests.js
+      isModuleBuild: !!argv.esm, // Used by skip matchers in testing/test-config.js
     };
   }
 

build-system/test-configs/config.js

@@ -24,7 +24,7 @@
  *   watched: boolean
  * }>
  */
-const initTestsPath = ['test/_init_tests.js'];
+const initTestsPath = ['testing/_init_tests.js'];
 
 const karmaHtmlFixturesPath = 'test/fixtures/*.html';
 

build-system/test-configs/forbidden-terms.js

@@ -96,8 +96,13 @@ const forbiddenTermsGlobal = {
       'css/ampshared.css',
     ],
   },
-  'describe\\.only': '',
-  'describes.*\\.only': '',
+  'describe\\.only': {
+    message: 'Please remove all instances of describe.only',
+    allowlist: ['testing/describes.js'],
+  },
+  'describes.*\\.only': {
+    message: 'Please remove all instances of describes.only',
+  },
   'dev\\(\\)\\.assert\\(': 'Use the devAssert function instead.',
   '[^.]user\\(\\)\\.assert\\(': 'Use the userAssert function instead.',
   'it\\.only': '',
@@ -211,6 +216,7 @@ const forbiddenTermsGlobal = {
       'src/amp-shadow.js',
       'src/inabox/amp-inabox.js',
       'src/service/ampdoc-impl.js',
+      'testing/_init_tests.js',
       'testing/describes.js',
       'testing/iframe.js',
     ],
@@ -323,6 +329,7 @@ const forbiddenTermsGlobal = {
       'src/log.js',
       'src/web-worker/web-worker.js',
       'tools/experiments/experiments.js',
+      'testing/_init_tests.js',
     ],
   },
   'parseUrlWithA': {
@@ -451,6 +458,7 @@ const forbiddenTermsGlobal = {
       'src/experiments.js',
       'src/service/cid-impl.js',
       'src/service/storage-impl.js',
+      'testing/_init_tests.js',
       'testing/fake-dom.js',
     ],
   },
@@ -597,6 +605,7 @@ const forbiddenTermsGlobal = {
       'and getMode() to access config',
     allowlist: [
       'build-system/externs/amp.extern.js',
+      'build-system/server/amp4test.js',
       'build-system/server/app.js',
       'build-system/tasks/e2e/index.js',
       'build-system/tasks/firebase.js',
@@ -613,8 +622,8 @@ const forbiddenTermsGlobal = {
       'src/experiments.js',
       'src/mode.js',
       'src/web-worker/web-worker.js', // Web worker custom error reporter.
+      'testing/_init_tests.js',
       'tools/experiments/experiments.js',
-      'build-system/server/amp4test.js',
     ],
   },
   'data:image/svg(?!\\+xml;charset=utf-8,)[^,]*,': {
@@ -647,7 +656,7 @@ const forbiddenTermsGlobal = {
       'Use of `this.skip()` is forbidden in test files. Use ' +
       '`this.skipTest()` from within a `before()` block instead. See #17245.',
     checkInTestFolder: true,
-    allowlist: ['test/_init_tests.js'],
+    allowlist: ['testing/_init_tests.js'],
   },
   '[^\\.]makeBodyVisible\\(': {
     message:
@@ -685,7 +694,7 @@ const forbiddenTermsGlobal = {
       'build-system/server/app-index/test/test-self.js',
       'build-system/server/app-index/test/test-template.js',
       'build-system/server/app-index/test/test.js',
-      'test/_init_tests.js',
+      'testing/_init_tests.js',
       'test/e2e/test-controller-promise.js',
       'test/e2e/test-expect.js',
       'validator/js/engine/amp4ads-parse-css_test.js',

contributing/build-on-duty.md

@@ -17,7 +17,7 @@ Make sure you are a member of the [#contributing](https://amphtml.slack.com/mess
             - Create a ["Related to: Flaky Tests" issue](https://github.com/ampproject/amphtml/issues?q=is%3Aopen+is%3Aissue+label%3A%22Related+to%3A+Flaky+Tests%22). **Make sure to find an appropriate owner for the issue and assign it to them.**
             - If needed, send a PR to disable the flaky test:
                 - For a normal `describe` test add [`.skip()`](https://mochajs.org/#inclusive-tests)
-                - For an integration test failing on a specific browser, add the corresponding `skip` function (e.g. `skipEdge()`). See the `skipXXX` functions in [\_init_tests.js](https://github.com/ampproject/amphtml/blob/main/test/_init_tests.js) for details.
+                - For an integration test failing on a specific browser, add the corresponding `skip` function (e.g. `skipEdge()`). See the `skipXXX` functions in [testing/test-config.js](https://github.com/ampproject/amphtml/blob/main/testing/test-config.js) for details.
             - Restart the failing parts of the build build on CircleCI by clicking the `Rerun workflow from failed` button on the build page (you must be signed into GitHub).
         - If the issue is due to a real breakage, work with the appropriate owner to rollback the offending PR. Rollbacks are preferable to fixes because fixes can often cause their own breakages.
 2. Keep an eye on incoming [Renovate PRs](https://github.com/ampproject/amphtml/pulls/renovate-bot), which result from an automated process to update our dependencies.

test/integration/test-toggle-display.js

@@ -17,48 +17,50 @@
 import {BrowserController} from '../../testing/test-helper';
 import {setInitialDisplay, toggle} from '../../src/style';
 
-describes.integration(
-  'toggle display helper',
-  {
-    enableIe: true,
-    body:
-      '<amp-img src="/examples/img/[email protected]" width="289" height="216"></amp-img>',
-  },
-  (env) => {
-    let browser, doc;
-    let img;
+describes.integration
+  .configure()
+  .enableIe()
+  .run(
+    'toggle display helper',
+    {
+      body:
+        '<amp-img src="/examples/img/[email protected]" width="289" height="216"></amp-img>',
+    },
+    (env) => {
+      let browser, doc;
+      let img;
 
-    beforeEach(async () => {
-      const {win} = env;
-      doc = win.document;
-      browser = new BrowserController(win);
+      beforeEach(async () => {
+        const {win} = env;
+        doc = win.document;
+        browser = new BrowserController(win);
 
-      await browser.waitForElementLayout('amp-img');
-      img = doc.querySelector('amp-img');
-    });
+        await browser.waitForElementLayout('amp-img');
+        img = doc.querySelector('amp-img');
+      });
 
-    function expectToggleDisplay(el) {
-      toggle(el, false);
-      expect(el).to.have.display('none');
-      toggle(el, true);
-      expect(el).to.not.have.display('none');
-    }
+      function expectToggleDisplay(el) {
+        toggle(el, false);
+        expect(el).to.have.display('none');
+        toggle(el, true);
+        expect(el).to.not.have.display('none');
+      }
 
-    it('toggles regular display', () => {
-      expectToggleDisplay(img);
-    });
+      it('toggles regular display', () => {
+        expectToggleDisplay(img);
+      });
 
-    it('toggles initial display style', () => {
-      setInitialDisplay(img, 'inline-block');
-      expectToggleDisplay(img);
-    });
+      it('toggles initial display style', () => {
+        setInitialDisplay(img, 'inline-block');
+        expectToggleDisplay(img);
+      });
 
-    it('toggles stylesheet display style', () => {
-      const style = doc.createElement('style');
-      style.innerText = 'amp-img { display: inline-block !important; }';
-      doc.head.appendChild(style);
+      it('toggles stylesheet display style', () => {
+        const style = doc.createElement('style');
+        style.innerText = 'amp-img { display: inline-block !important; }';
+        doc.head.appendChild(style);
 
-      expectToggleDisplay(img);
-    });
-  }
-);
+        expectToggleDisplay(img);
+      });
+    }
+  );

test/unit/test-describes.js

@@ -33,23 +33,35 @@ describe('fetch-mock', () => {
     });
   }
 
-  describes.realWin(
-    'on realWin',
-    {
-      mockFetch: true,
-    },
-    (env) => {
-      runTests(env);
-    }
-  );
-
-  describes.fakeWin(
-    'on fakeWin',
-    {
-      mockFetch: true,
-    },
-    (env) => {
-      runTests(env);
-    }
-  );
+  describes.realWin('on realWin', {mockFetch: true}, (env) => {
+    runTests(env);
+  });
+
+  describes.fakeWin('on fakeWin', {mockFetch: true}, (env) => {
+    runTests(env);
+  });
+});
+
+function runConfiguredDescribe() {
+  describe.configure().run('configure describe', () => {
+    it.configure().run('configure it', () => {
+      expect(2 + 2).to.equal(4);
+    });
+  });
+}
+
+describes.sandboxed.configure().run('configure sandboxed', {}, () => {
+  runConfiguredDescribe();
+});
+
+describes.fakeWin.configure().run('configure fakeWin', {}, () => {
+  runConfiguredDescribe();
+});
+
+describes.realWin.configure().run('configure realWin', {}, () => {
+  runConfiguredDescribe();
+});
+
+describes.integration.configure().run('configure integration', {}, () => {
+  runConfiguredDescribe();
 });

testing/.eslintrc.js

@@ -19,5 +19,6 @@ module.exports = {
     'local/no-export-side-effect': 0,
     'local/no-module-exports': 0,
     'local/no-style-property-setting': 0,
+    'local/no-spread': 0,
   },
 };