tests: Use require() to load frontend test specs

This makes core and plugin tests consistent with each other, makes it
possible to `require()` relative paths in spec files, simplifies the
code somewhat, and should make it easier to move away from
require-kernel.

Also:
  * Wrap plugin tests inside a `describe()` that contains the plugin
    name to make it easier to grep for a plugin's tests and for
    consistency with core tests.
  * Add "<core>" to the core test descriptions to make it easier to
    distinguish them from plugin tests.

src/node/hooks/express/tests.js

@@ -31,19 +31,29 @@ const findSpecs = async (specDir) => {
 
 exports.expressCreateServer = (hookName, args, cb) => {
   args.app.get('/tests/frontend/frontendTestSpecs.js', async (req, res) => {
-    const [coreTests, pluginTests] = await Promise.all([getCoreTests(), getPluginTests()]);
+    const modules = [];
-
+    await Promise.all(Object.entries(plugins.plugins).map(async ([plugin, def]) => {
-    // merge the two sets of results
+      let {package: {path: pluginPath}} = def;
-    let files = [].concat(coreTests, pluginTests).sort();
+      if (!pluginPath.endsWith(path.sep)) pluginPath += path.sep;
-
+      const specDir = `${plugin === 'ep_etherpad-lite' ? '' : 'static/'}tests/frontend/specs`;
-    // remove admin tests if the setting to enable them isn't in settings.json
+      for (const spec of await findSpecs(path.join(pluginPath, specDir))) {
-    if (!settings.enableAdminUITests) {
+        if (plugin === 'ep_etherpad-lite' && !settings.enableAdminUITests &&
-      files = files.filter((file) => file.indexOf('admin') !== 0);
+            spec.startsWith('admin')) continue;
-    }
+        modules.push(`${plugin}/${specDir}/${spec.replace(/\.js$/, '')}`);
-
+      }
-    console.debug('Sent browser the following test specs:', files);
+    }));
+    // Sort plugin tests before core tests.
+    modules.sort((a, b) => {
+      a = String(a);
+      b = String(b);
+      const aCore = a.startsWith('ep_etherpad-lite/');
+      const bCore = b.startsWith('ep_etherpad-lite/');
+      if (aCore === bCore) return a.localeCompare(b);
+      return aCore ? 1 : -1;
+    });
+    console.debug('Sent browser the following test spec modules:', modules);
     res.setHeader('content-type', 'application/javascript');
-    res.end(`window.frontendTestSpecs = ${JSON.stringify(files, null, 2)};\n`);
+    res.end(`window.frontendTestSpecs = ${JSON.stringify(modules, null, 2)};\n`);
   });
 
   const rootTestFolder = path.join(settings.root, 'src/tests/frontend/');
@@ -57,16 +67,9 @@ exports.expressCreateServer = (hookName, args, cb) => {
   // version used with Express v4.x) interprets '.' and '*' differently than regexp.
   args.app.get('/tests/frontend/:file([\\d\\D]{0,})', (req, res, next) => {
     (async () => {
-      let relFile = sanitizePathname(req.params.file);
+      let file = sanitizePathname(req.params.file);
-      if (['', '.', './'].includes(relFile)) relFile = 'index.html';
+      if (['', '.', './'].includes(file)) file = 'index.html';
-      const file = path.join(rootTestFolder, relFile);
+      res.sendFile(path.join(rootTestFolder, file));
-      if (relFile.startsWith('specs/') && file.endsWith('.js')) {
-        const content = await fsp.readFile(file);
-        res.setHeader('content-type', 'application/javascript');
-        res.send(`describe(${JSON.stringify(path.basename(file))}, function () {\n${content}\n});`);
-      } else {
-        res.sendFile(file);
-      }
     })().catch((err) => next(err || new Error(err)));
   });
 
@@ -76,16 +79,3 @@ exports.expressCreateServer = (hookName, args, cb) => {
 
   return cb();
 };
-
-const getPluginTests = async (callback) => {
-  const specPath = 'static/tests/frontend/specs';
-  const specLists = await Promise.all(Object.entries(plugins.plugins).map(async ([plugin, def]) => {
-    if (plugin === 'ep_etherpad-lite') return [];
-    const {package: {path: pluginPath}} = def;
-    const specs = await findSpecs(path.join(pluginPath, specPath));
-    return specs.map((spec) => `/static/plugins/${plugin}/${specPath}/${spec}`);
-  }));
-  return [].concat(...specLists);
-};
-
-const getCoreTests = async () => await findSpecs('src/tests/frontend/specs');

src/node/utils/Minify.js

@@ -165,6 +165,8 @@ const minify = async (req, res) => {
       filename = path.join('../node_modules/', library, libraryPath);
     }
   }
+  const [, spec] = /^plugins\/ep_etherpad-lite\/(tests\/frontend\/specs\/.*)/.exec(filename) || [];
+  if (spec != null) filename = `../${spec}`;
 
   const contentType = mime.lookup(filename);
 

src/tests/frontend/runner.js

@@ -141,9 +141,17 @@ $(() => {
   require.setLibraryURI(absUrl('../../javascripts/lib'));
   require.setGlobalKeyPath('require');
 
-  const $body = $('body');
+  // This loads the test specs serially. While it is technically possible to load them in parallel,
-  for (const spec of window.frontendTestSpecs.map((spec) => encodeURI(spec))) {
+  // the code would be very complex (it involves wrapping require.define(), configuring
-    $body.append($('<script>').attr('src', spec.startsWith('/') ? spec : `specs/${spec}`));
+  // require-kernel to use the wrapped .define() via require.setGlobalKeyPath(), and using the
+  // asynchronous form of require()). In addition, the performance gains would be minimal because
+  // require-kernel only loads 2 at a time by default. (Increasing the default could cause problems
+  // because browsers like to limit the number of concurrent fetches.)
+  for (const spec of window.frontendTestSpecs) {
+    const desc = spec
+        .replace(/^ep_etherpad-lite\/tests\/frontend\/specs\//, '<core> ')
+        .replace(/^([^/ ]*)\/static\/tests\/frontend\/specs\//, '<$1> ');
+    describe(`${desc}.js`, function () { require(spec); });
   }
 
   // initialize the test helper