From 311fe41253b79349e62e829dab46b7743ff5d756 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 23 Aug 2022 09:43:19 +0100 Subject: [PATCH 1/4] DEV: Skip loading plugin JS when running only core tests Plugins often change core behavior, and thereby cause core's tests to fail. In CI, we work around this problem by running core CI without any plugins loaded. In development, the only option to safely run the core tests is to uninstall all plugins, which is clearly a bad developer experience. This commit aims to improve that experience. The `qunit_skip_plugins=1` flag would previously prevent the plugin **tests** from running. This commit extends that flag to also affect the plugin's application JS. --- .../discourse/lib/bootstrap-json/index.js | 18 ++++++++----- .../scripts/discourse-test-load-dynamic-js.js | 25 +++++++++++++++++++ .../discourse-test-trigger-ember-cli-boot.js | 1 + .../javascripts/discourse/tests/index.html | 25 ++++++++++++------- .../discourse/tests/test-boot-ember-cli.js | 7 ++++++ 5 files changed, 61 insertions(+), 15 deletions(-) create mode 100644 app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js create mode 100644 app/assets/javascripts/discourse/public/assets/scripts/discourse-test-trigger-ember-cli-boot.js diff --git a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js index 0eb9ce0b3decec..3249cbbc6ebd7e 100644 --- a/app/assets/javascripts/discourse/lib/bootstrap-json/index.js +++ b/app/assets/javascripts/discourse/lib/bootstrap-json/index.js @@ -388,24 +388,30 @@ module.exports = { for (const { name, hasJs } of pluginInfos) { if (hasJs) { - scripts.push(`plugins/${name}.js`); + scripts.push({ src: `plugins/${name}.js`, name }); } if (fs.existsSync(`../plugins/${name}_extras.js.erb`)) { - scripts.push(`plugins/${name}_extras.js`); + scripts.push({ src: `plugins/${name}_extras.js`, name }); } } } else { - scripts.push("discourse/tests/active-plugins.js"); + scripts.push({ + src: "discourse/tests/active-plugins.js", + name: "_all", + }); } - scripts.push("admin-plugins.js"); + scripts.push({ src: "admin-plugins.js", name: "_admin" }); return scripts - .map((s) => ``) + .map( + ({ src, name }) => + `` + ) .join("\n"); } else if (shouldLoadPluginTestJs() && type === "test-plugin-tests-js") { - return ``; + return ``; } }, diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js new file mode 100644 index 00000000000000..31f1fc01488753 --- /dev/null +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js @@ -0,0 +1,25 @@ +const dynamicJsTemplate = document.querySelector("#dynamic-test-js"); + +const params = new URLSearchParams(document.location.search); +const skipPlugins = params.get("qunit_skip_plugins"); + +for(const element of Array.from(dynamicJsTemplate.content.childNodes)){ + const clone = element.cloneNode(true); + + if(skipPlugins && clone.dataset?.discoursePlugin){ + continue; + } + + if(clone.tagName === "SCRIPT" && clone.innerHTML.includes("EmberENV.TESTS_FILE_LOADED")){ + // Inline script introduced by ember-cli. Incompatible with CSP and our custom plugin JS loading system + // https://github.com/ember-cli/ember-cli/blob/04a38fda2c/lib/utilities/ember-app-utils.js#L131 + // We re-implement in test-boot-ember-cli.js + continue; + } + + if(clone.tagName=== "SCRIPT"){ + clone.async = false; + } + + document.body.appendChild(clone) +} diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-trigger-ember-cli-boot.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-trigger-ember-cli-boot.js new file mode 100644 index 00000000000000..65f629d8a026d8 --- /dev/null +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-trigger-ember-cli-boot.js @@ -0,0 +1 @@ +require("discourse/tests/test-boot-ember-cli"); diff --git a/app/assets/javascripts/discourse/tests/index.html b/app/assets/javascripts/discourse/tests/index.html index e8c2e10b163351..359b3caaa10c05 100644 --- a/app/assets/javascripts/discourse/tests/index.html +++ b/app/assets/javascripts/discourse/tests/index.html @@ -50,15 +50,22 @@ - {{content-for "test-plugin-js"}} - - - {{content-for "test-plugin-tests-js"}} - - - {{content-for "body-footer"}} {{content-for "test-body-footer"}} + + + + + + + + diff --git a/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js b/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js index c11fbcfeb57061..eabbec1ef7c85e 100644 --- a/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js +++ b/app/assets/javascripts/discourse/tests/test-boot-ember-cli.js @@ -8,6 +8,13 @@ import { setup } from "qunit-dom"; setEnvironment("testing"); document.addEventListener("discourse-booted", () => { + // eslint-disable-next-line no-undef + if (!EmberENV.TESTS_FILE_LOADED) { + throw new Error( + 'The tests file was not loaded. Make sure your tests index.html includes "assets/tests.js".' + ); + } + const script = document.getElementById("plugin-test-script"); if (script && !requirejs.entries["discourse/tests/plugin-tests"]) { throw new Error( From 1b73c8bb32ee7bd94d6b27e5c2470ec4af9d807d Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 23 Aug 2022 09:52:06 +0100 Subject: [PATCH 2/4] Cleanup and prettier --- .prettierignore | 1 + .../scripts/discourse-test-listen-boot.js | 2 +- .../scripts/discourse-test-load-dynamic-js.js | 17 ++++++++++------- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/.prettierignore b/.prettierignore index fff97dce442ea2..b33a5618e4d33a 100644 --- a/.prettierignore +++ b/.prettierignore @@ -17,6 +17,7 @@ lib/javascripts/messageformat.js lib/highlight_js/ plugins/**/lib/javascripts/locale public/ +!/app/assets/javascripts/discourse/public vendor/ app/assets/javascripts/discourse/tests/fixtures spec/ diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-listen-boot.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-listen-boot.js index 48a7c880f6b6bf..bc903572c7b33d 100644 --- a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-listen-boot.js +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-listen-boot.js @@ -27,4 +27,4 @@ document.body.insertAdjacentHTML( ` ); -require('discourse/tests/test-boot-ember-cli'); +require("discourse/tests/test-boot-ember-cli"); diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js index 31f1fc01488753..0839fb6558331e 100644 --- a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js @@ -3,23 +3,26 @@ const dynamicJsTemplate = document.querySelector("#dynamic-test-js"); const params = new URLSearchParams(document.location.search); const skipPlugins = params.get("qunit_skip_plugins"); -for(const element of Array.from(dynamicJsTemplate.content.childNodes)){ - const clone = element.cloneNode(true); - - if(skipPlugins && clone.dataset?.discoursePlugin){ +for (const element of Array.from(dynamicJsTemplate.content.childNodes)) { + if (skipPlugins && element.dataset?.discoursePlugin) { continue; } - if(clone.tagName === "SCRIPT" && clone.innerHTML.includes("EmberENV.TESTS_FILE_LOADED")){ + if ( + element.tagName === "SCRIPT" && + element.innerHTML.includes("EmberENV.TESTS_FILE_LOADED") + ) { // Inline script introduced by ember-cli. Incompatible with CSP and our custom plugin JS loading system // https://github.com/ember-cli/ember-cli/blob/04a38fda2c/lib/utilities/ember-app-utils.js#L131 // We re-implement in test-boot-ember-cli.js continue; } - if(clone.tagName=== "SCRIPT"){ + const clone = element.cloneNode(true); + + if (clone.tagName === "SCRIPT") { clone.async = false; } - document.body.appendChild(clone) + document.body.appendChild(clone); } From 5369efcdb376b4c44b93949cd21c89ea8100b05c Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 23 Aug 2022 10:02:51 +0100 Subject: [PATCH 3/4] Output templated content to dedicated element Makes it clear what's happened when looking at the HTML in the dev tools --- .../public/assets/scripts/discourse-test-load-dynamic-js.js | 2 +- app/assets/javascripts/discourse/tests/index.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js index 0839fb6558331e..89842d2c656afe 100644 --- a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js @@ -24,5 +24,5 @@ for (const element of Array.from(dynamicJsTemplate.content.childNodes)) { clone.async = false; } - document.body.appendChild(clone); + document.querySelector("discourse-dynamic-test-js").appendChild(clone); } diff --git a/app/assets/javascripts/discourse/tests/index.html b/app/assets/javascripts/discourse/tests/index.html index 359b3caaa10c05..03d82fa2db3fd1 100644 --- a/app/assets/javascripts/discourse/tests/index.html +++ b/app/assets/javascripts/discourse/tests/index.html @@ -64,7 +64,7 @@ - + From 392cc76e56edf6d988993bf48717e663cd21e0f5 Mon Sep 17 00:00:00 2001 From: David Taylor Date: Tue, 23 Aug 2022 10:08:26 +0100 Subject: [PATCH 4/4] Update app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js Co-authored-by: Jarek Radosz --- .../public/assets/scripts/discourse-test-load-dynamic-js.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js index 89842d2c656afe..fc0237c6225d3b 100644 --- a/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js +++ b/app/assets/javascripts/discourse/public/assets/scripts/discourse-test-load-dynamic-js.js @@ -3,7 +3,7 @@ const dynamicJsTemplate = document.querySelector("#dynamic-test-js"); const params = new URLSearchParams(document.location.search); const skipPlugins = params.get("qunit_skip_plugins"); -for (const element of Array.from(dynamicJsTemplate.content.childNodes)) { +for (const element of dynamicJsTemplate.content.childNodes) { if (skipPlugins && element.dataset?.discoursePlugin) { continue; }