FEATURE: Allow theme tests to be run in production (take 2) (PR #12845)

This PR is mostly the same as the original one FEATURE: Allow theme tests to be run in production by OsamaSayegh · Pull Request #12815 · discourse/discourse · GitHub with 3 new changes changes on top of the original PR (each new change is made as a new commit so they can be easily reviewed).

The new changes are:

(1) Fixing /theme-qunit so it works on subfolder installs and fixing lazy-loaded assets so they’re loaded from the CDN instead of hitting the app server.

(2) Create new theme_test_helper.js and theme_test_vendor.js and use them in /theme-qunit instead of test_helper.js. The reason for this change is test_helper.js is very large and precompiling it is so expensive (takes about 40 seconds and 1GB of RAM) that it can make small servers run out of memory. This is why the original PR was reverted: Failed to bootstrap due to out of memory killer - #18 by Falco - bug - Discourse Meta.

(3) Add unsafe-eval to CSP for /theme-qunit route.


(copied from FIX: Make theme tests work with subfolder and fetch lazy-loaded assets from CDN by OsamaSayegh · Pull Request #12837 · discourse/discourse · GitHub)

Not a huge fan of this hack, but setting baseUri in lib/get-url (which is needed to get the right CDN URL) makes our ajax lib prepend the subfolder path to all the requests it makes which makes pretender unable to recognize our fixtures. One alternative fix is to add an isTesting() check in our ajax lib here and if we’re in test mode then we skip the getURL call, but I’m not a fan of that approach either.

I don’t quite understand this change. Pretender assumes there is no subfolder because our tests do that. Are there some pretender tests that you’d like to work with subfolder?

In principal this seems OK, but I am curious if you considered not running terser or whatever is slow on this bundle? Since it’s only used for tests I don’t think we need to do all the compilation and could get away with whatever we use in development mode.

Before the original PR was reverted, I tried running a theme’s tests on Meta but the tests kept freezing and never finished. After some debugging I realized that it was hanging because at some point during the tests Discourse tried to fetch jquery.magnific-popup.min.js (which is lazy-loaded) but it failed with 404 because it hit meta.discourse.org instead of the configured CDN.

Now that I’ve thought this a little bit more, I see we have 3 solutions:

  1. Configure CDN when running theme tests so that lazy-loaded assets are loaded exactly like how we do for the rest of the app, but this means we have to also configure subfolder (because getting the right CDN URL requires the subfolder bit). This is what I’ve done here, but configuring subfolder will make our ajax prepend subfolder path to requests paths and that breaks pretender (because it would no longer recognize e.g. /subfolder/latest.json)

  2. In test mode we ignore the CDN and load directly from the app. E.g., instead of loading from https://d3bpeqsaub0i6y.cloudfront.net/javascripts/magnific-popup/1.1.0/jquery.magnific-popup.min.js we load directly from meta https://meta.discourse.org/javascripts/magnific-popup/1.1.0/jquery.magnific-popup.min.js.

  3. Don’t lazy-load anything in tests and include everything tests could possibly need up front. This means we need to add an isTesting check in loadScript and if we’re in test mode then we don’t attempt to load anything.

with whatever we use in development mode.

I don’t think that’s possible because IIRC in production we can’t do on-the-fly compilation like development (or it’s turned off at least) and also I think the NGINX that we include in our docker image takes care of handling /assets/* requests and they never hit the Rails app.

I’ve measured how much it takes to precompile the tests bundle now and it’s about ~10 seconds which is not great, but it’s lower than application.js which takes ~11 seconds on my machine. For comparison, the tests bundle in the original PR took more than 40 seconds.

Sounds good to me, thanks. Let’s merge carefully due to new release!