FEATURE: Introduce theme/component QUnit tests (take 2) (#12661)

FEATURE: Introduce theme/component QUnit tests (take 2) (#12661)

This commit allows themes and theme components to have QUnit tests. To add tests to your theme/component, create a top-level directory in your theme and name it test, and Discourse will save all the files in that directory (and its sub-directories) as “tests files” in the database. While tests files/directories are not required to be organized in a specific way, we recommend that you follow Discourse core’s tests structure.

Writing theme tests should be identical to writing plugins or core tests; all the import statements and APIs that you see in core (or plugins) to define/setup tests should just work in themes.

You do need a working Discourse install to run theme tests, and you have 2 ways to run theme tests:

  • In the browser at the /qunit route. /qunit will run tests of all active themes/components as well as core and plugins. The /qunit now accepts a theme_name or theme_url params that you can use to run tests of a specific theme/component like so: /qunit?theme_name=<your_theme_name>.

  • In the command line using the themes:qunit rake task. This take is meant to run tests of a single theme/component so you need to provide it with a theme name or URL like so: bundle exec rake themes:qunit[name=<theme_name>] or bundle exec rake themes:qunit[url=<theme_url>].

There are some refactors to how Discourse processes JavaScript that comes with themes/components, and these refactors may break your JS customizations; see Upcoming core changes that may break some themes/components (April 12) - dev - Discourse Meta for details on how you can check if your themes/components are affected and what you need to do to fix them.

This commit also improves theme error handling in Discourse. We will now be able to catch errors that occur when theme initializers are run and prevent them from breaking the site and other themes/components.

diff --git a/app/assets/javascripts/discourse/app/app.js b/app/assets/javascripts/discourse/app/app.js
index b28effd..ddcc134 100644
--- a/app/assets/javascripts/discourse/app/app.js
+++ b/app/assets/javascripts/discourse/app/app.js
@@ -1,8 +1,10 @@
 import Application from "@ember/application";
 import Mousetrap from "mousetrap";
 import { buildResolver } from "discourse-common/resolver";
+import { isTesting } from "discourse-common/config/environment";
 
 const _pluginCallbacks = [];
+let _themeErrors = [];
 
 const Discourse = Application.extend({
   rootElement: "#main",
@@ -19,14 +21,37 @@ const Discourse = Application.extend({
   Resolver: buildResolver("discourse"),
 
   _prepareInitializer(moduleName) {
-    const module = requirejs(moduleName, null, null, true);
-    if (!module) {
-      throw new Error(moduleName + " must export an initializer.");
+    const themeId = moduleThemeId(moduleName);
+    let module = null;
+
+    try {
+      module = requirejs(moduleName, null, null, true);
+
+      if (!module) {
+        throw new Error(moduleName + " must export an initializer.");
+      }
+    } catch (err) {
+      if (!themeId || isTesting()) {
+        throw err;
+      }
+      _themeErrors.push([themeId, err]);
+      fireThemeErrorEvent();
+      return;
     }
 
     const init = module.default;
     const oldInitialize = init.initialize;
-    init.initialize = (app) => oldInitialize.call(init, app.__container__, app);
+    init.initialize = (app) => {
+      try {
+        return oldInitialize.call(init, app.__container__, app);
+      } catch (err) {
+        if (!themeId || isTesting()) {
+          throw err;
+        }
+        _themeErrors.push([themeId, err]);
+        fireThemeErrorEvent();
+      }
+    };
 
     return init;
   },
@@ -37,9 +62,15 @@ const Discourse = Application.extend({
 
     Object.keys(requirejs._eak_seen).forEach((key) => {
       if (/\/pre\-initializers\//.test(key)) {
-        this.initializer(this._prepareInitializer(key));
+        const initializer = this._prepareInitializer(key);
+        if (initializer) {
+          this.initializer(initializer);
+        }
       } else if (/\/(api\-)?initializers\//.test(key)) {
-        this.instanceInitializer(this._prepareInitializer(key));
+        const initializer = this._prepareInitializer(key);
+        if (initializer) {
+          this.instanceInitializer(initializer);
+        }
       }
     });
 
@@ -60,4 +91,22 @@ const Discourse = Application.extend({
   },
 });
 
+function moduleThemeId(moduleName) {
+  const match = moduleName.match(/^discourse\/theme\-(\d+)\//);
+  if (match) {
+    return parseInt(match[1], 10);
+  }
+}
+
+function fireThemeErrorEvent() {
+  const event = new CustomEvent("discourse-theme-error");
+  document.dispatchEvent(event);
+}
+
+export function getAndClearThemeErrors() {
+  const copy = _themeErrors;
+  _themeErrors = [];
+  return copy;
+}
+
 export default Discourse;
diff --git a/app/assets/javascripts/discourse/app/helpers/theme-helpers.js b/app/assets/javascripts/discourse/app/helpers/theme-helpers.js
index 9898775..477b89b 100644
--- a/app/assets/javascripts/discourse/app/helpers/theme-helpers.js
+++ b/app/assets/javascripts/discourse/app/helpers/theme-helpers.js
@@ -1,6 +1,7 @@
-import { helperContext, registerUnbound } from "discourse-common/lib/helpers";
+import { registerUnbound } from "discourse-common/lib/helpers";
 import I18n from "I18n";
 import deprecated from "discourse-common/lib/deprecated";
+import { getSetting as getThemeSetting } from "discourse/lib/theme-settings-store";
 
 registerUnbound("theme-i18n", (themeId, key, params) => {
   return I18n.t(`theme_translations.${themeId}.${key}`, params);
@@ -18,5 +19,6 @@ registerUnbound("theme-setting", (themeId, key, hash) => {
       { since: "v2.2.0.beta8", dropFrom: "v2.3.0" }
     );
   }
-  return helperContext().themeSettings.getSetting(themeId, key);
+
+  return getThemeSetting(themeId, key);
 });
diff --git a/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js b/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js
index b985d85..fa2ca51 100644
--- a/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js
+++ b/app/assets/javascripts/discourse/app/initializers/auto-load-modules.js
@@ -19,7 +19,6 @@ export function autoLoadModules(container, registry) {
 
   let context = {
     siteSettings: container.lookup("site-settings:main"),
-    themeSettings: container.lookup("service:theme-settings"),
     keyValueStore: container.lookup("key-value-store:main"),
     capabilities: container.lookup("capabilities:main"),
     currentUser: container.lookup("current-user:main"),
diff --git a/app/assets/javascripts/discourse/app/lib/theme-settings-store.js b/app/assets/javascripts/discourse/app/lib/theme-settings-store.js
new file mode 100644
index 0000000..6503d6b
--- /dev/null
+++ b/app/assets/javascripts/discourse/app/lib/theme-settings-store.js
@@ -0,0 +1,47 @@
+import { get } from "@ember/object";
+
+const originalSettings = {};
+const settings = {};
+
+export function registerSettings(
+  themeId,
+  settingsObject,
+  { force = false } = {}
+) {
+  if (settings[themeId] && !force) {
+    return;
+  }
+  originalSettings[themeId] = Object.assign({}, settingsObject);
+  const s = {};
+  Object.keys(settingsObject).forEach((key) => {
+    Object.defineProperty(s, key, {
+      enumerable: true,
+      get() {
+        return settingsObject[key];
+      },
+      set(newVal) {
+        settingsObject[key] = newVal;
+      },
+    });
+  });
+  settings[themeId] = s;
+}
+
+export function getSetting(themeId, settingKey) {
+  if (settings[themeId]) {
+    return get(settings[themeId], settingKey);
+  }
+  return null;
+}
+
+export function getObjectForTheme(themeId) {
+  return settings[themeId];
+}
+
+export function resetSettings() {
+  Object.keys(originalSettings).forEach((themeId) => {
+    Object.keys(originalSettings[themeId]).forEach((key) => {
+      settings[themeId][key] = originalSettings[themeId][key];
+    });
+  });
+}
diff --git a/app/assets/javascripts/discourse/app/lib/utilities.js b/app/assets/javascripts/discourse/app/lib/utilities.js
index f7b7bad..c635edc 100644
--- a/app/assets/javascripts/discourse/app/lib/utilities.js
+++ b/app/assets/javascripts/discourse/app/lib/utilities.js
@@ -1,6 +1,5 @@
 import getURL, { getURLWithCDN } from "discourse-common/lib/get-url";
 import Handlebars from "handlebars";
-import I18n from "I18n";
 import { deepMerge } from "discourse-common/lib/object";
 import { escape } from "pretty-text/sanitizer";
 import { helperContext } from "discourse-common/lib/helpers";
@@ -444,40 +443,6 @@ export function postRNWebviewMessage(prop, value) {
   }
 }
 
-function reportToLogster(name, error) {
-  const data = {
-    message: `${name} theme/component is throwing errors`,
-    stacktrace: error.stack,
-  };
-
-  Ember.$.ajax(getURL("/logs/report_js_error"), {
-    data,
-    type: "POST",
-    cache: false,
-  });
-}
-// this function is used in lib/theme_javascript_compiler.rb
-export function rescueThemeError(name, error, api) {
-  /* eslint-disable-next-line no-console */
-  console.error(`"${name}" error:`, error);
-  reportToLogster(name, error);
-
-  const currentUser = api.getCurrentUser();
-  if (!currentUser || !currentUser.admin) {
-    return;
-  }
-
-  const path = getURL(`/admin/customize/themes`);
-  const message = I18n.t("themes.broken_theme_alert", {
-    theme: name,
-    path: `<a href="${path}">${path}</a>`,
-  });
-  const alertDiv = document.createElement("div");
-  alertDiv.classList.add("broken-theme-alert");
-  alertDiv.innerHTML = `⚠️ ${message}`;
-  document.body.prepend(alertDiv);
-}
-
 const CODE_BLOCKS_REGEX = /^(    |\t).*|`[^`]+`|^`‍``[^]*?^`‍``|\[code\][^]*?\[\/code\]/gm;
 //                        |      ^     |   ^   |      ^      |           ^           |

[... diff too long, it was truncated ...]

GitHub sha: cd24eff5

This commit appears in #12661 which was approved by eviltrout. It was merged by OsamaSayegh.