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

This commit allows site admins to run theme tests in production via a new `/theme-qunit` route. When you visit `/theme-qunit`, you'll see a list of the themes/components installed on your site that have tests, and from there you can select a theme or component that you run its tests.

We also have a new rake task `themes:install_and_test` that can be used to install a list of themes/components on a temporary database and run the tests of the themes/components that are installed. This rake task can be useful when upgrading/deploying a Discourse instance to make sure that the installed themes/components are compatible with the new Discourse version being deployed, and if the tests fail you can abort the build/deploy process so you don't end up with a broken site.

spec/models/theme_spec.rb

@@ -865,4 +865,41 @@ HTML
     end
   end
 
+  describe "#baked_js_tests_with_digest" do
+    before do
+      ThemeField.create!(
+        theme_id: theme.id,
+        target_id: Theme.targets[:settings],
+        name: "yaml",
+        value: "some_number: 1"
+      )
+      theme.set_field(
+        target: :tests_js,
+        type: :js,
+        name: "acceptance/some-test.js",
+        value: "assert.ok(true);"
+      )
+      theme.save!
+    end
+
+    it 'returns nil for content and digest if theme does not have tests' do
+      ThemeField.destroy_all
+      expect(theme.baked_js_tests_with_digest).to eq([nil, nil])
+    end
+
+    it 'digest does not change when settings are changed' do
+      content, digest = theme.baked_js_tests_with_digest
+      expect(content).to be_present
+      expect(digest).to be_present
+      expect(content).to include("assert.ok(true);")
+
+      theme.update_setting(:some_number, 55)
+      theme.save!
+      expect(theme.build_settings_hash[:some_number]).to eq(55)
+
+      new_content, new_digest = theme.baked_js_tests_with_digest
+      expect(new_content).to eq(content)
+      expect(new_digest).to eq(digest)
+    end
+  end
 end

spec/requests/qunit_controller_spec.rb

@@ -3,74 +3,102 @@
 require 'rails_helper'
 
 describe QunitController do
-  let(:theme) { Fabricate(:theme, name: 'main-theme') }
-  let(:component) { Fabricate(:theme, component: true, name: 'enabled-component') }
-  let(:disabled_component) { Fabricate(:theme, component: true, enabled: false, name: 'disabled-component') }
+  describe "#theme" do
+    let(:theme) { Fabricate(:theme, name: 'main-theme') }
+    let(:component) { Fabricate(:theme, component: true, name: 'enabled-component') }
+    let(:disabled_component) { Fabricate(:theme, component: true, enabled: false, name: 'disabled-component') }
+    let(:theme_without_tests) { Fabricate(:theme, name: 'no-tests-guy') }
 
-  before do
-    Theme.destroy_all
-    theme.set_default!
-    component.add_relative_theme!(:parent, theme)
-    disabled_component.add_relative_theme!(:parent, theme)
-    [theme, component, disabled_component].each do |t|
-      t.set_field(
-        target: :extra_js,
-        type: :js,
-        name: "discourse/initializers/my-#{t.id}-initializer.js",
-        value: "console.log(#{t.id});"
-      )
-      t.set_field(
-        target: :tests_js,
-        type: :js,
-        name: "acceptance/some-test-#{t.id}.js",
-        value: "assert.ok(#{t.id});"
-      )
-      t.save!
+    before do
+      Theme.destroy_all
+      theme.set_default!
+      component.add_relative_theme!(:parent, theme)
+      disabled_component.add_relative_theme!(:parent, theme)
+      [theme, component, disabled_component].each do |t|
+        t.set_field(
+          target: :extra_js,
+          type: :js,
+          name: "discourse/initializers/my-#{t.id}-initializer.js",
+          value: "console.log(#{t.id});"
+        )
+        t.set_field(
+          target: :tests_js,
+          type: :js,
+          name: "acceptance/some-test-#{t.id}.js",
+          value: "assert.ok(#{t.id});"
+        )
+        t.save!
+      end
     end
-  end
 
-  context "when no theme is specified" do
-    it "includes tests of enabled theme + components" do
-      get '/qunit'
-      js_urls = JavascriptCache.where(theme_id: [theme.id, component.id]).map(&:url)
-      expect(js_urls.size).to eq(2)
-      js_urls.each do |url|
-        expect(response.body).to include(url)
-      end
-      [theme, component].each do |t|
-        expect(response.body).to include("/theme-javascripts/tests/#{t.id}.js")
+    context "non-admin users on production" do
+      before do
+        Rails.env.stubs(:production?).returns(true)
       end
 
-      js_urls = JavascriptCache.where(theme_id: disabled_component).map(&:url)
-      expect(js_urls.size).to eq(1)
-      js_urls.each do |url|
-        expect(response.body).not_to include(url)
+      it "anons cannot see the page" do
+        get '/theme-qunit'
+        expect(response.status).to eq(404)
+      end
+
+      it "regular users cannot see the page" do
+        sign_in(Fabricate(:user))
+        get '/theme-qunit'
+        expect(response.status).to eq(404)
       end
-      expect(response.body).not_to include("/theme-javascripts/tests/#{disabled_component.id}.js")
     end
-  end
 
-  context "when a theme is specified" do
-    it "includes tests of the specified theme only" do
-      [theme, disabled_component].each do |t|
-        get "/qunit?theme_name=#{t.name}"
-        js_urls = JavascriptCache.where(theme_id: t.id).map(&:url)
-        expect(js_urls.size).to eq(1)
-        js_urls.each do |url|
-          expect(response.body).to include(url)
-        end
-        expect(response.body).to include("/theme-javascripts/tests/#{t.id}.js")
+    context "admin users" do
+      before do
+        sign_in(Fabricate(:admin))
+      end
 
-        excluded = Theme.pluck(:id) - [t.id]
-        js_urls = JavascriptCache.where(theme_id: excluded).map(&:url)
-        expect(js_urls.size).to eq(2)
-        js_urls.each do |url|
-          expect(response.body).not_to include(url)
-        end
-        excluded.each do |id|
-          expect(response.body).not_to include("/theme-javascripts/tests/#{id}.js")
+      context "when no theme is specified" do
+        it "renders a list of themes and components that have tests" do
+          get '/theme-qunit'
+          expect(response.status).to eq(200)
+          [theme, component, disabled_component].each do |t|
+            expect(response.body).to include(t.name)
+            expect(response.body).to include("/theme-qunit?id=#{t.id}")
+          end
+          expect(response.body).not_to include(theme_without_tests.name)
+          expect(response.body).not_to include("/theme-qunit?id=#{theme_without_tests.id}")
         end
       end
+
+      it "can specify theme by id" do
+        get "/theme-qunit?id=#{theme.id}"
+        expect(response.status).to eq(200)
+        expect(response.body).to include("/theme-javascripts/tests/#{theme.id}-")
+      end
+
+      it "can specify theme by name" do
+        get "/theme-qunit?name=#{theme.name}"
+        expect(response.status).to eq(200)
+        expect(response.body).to include("/theme-javascripts/tests/#{theme.id}-")
+      end
+
+      it "can specify theme by url" do
+        theme.build_remote_theme(remote_url: "git@github.com:discourse/discourse.git").save!
+        theme.save!
+        get "/theme-qunit?url=#{theme.remote_theme.remote_url}"
+        expect(response.status).to eq(200)
+        expect(response.body).to include("/theme-javascripts/tests/#{theme.id}-")
+      end
+
+      it "themes qunit page includes all the JS/CSS it needs" do
+        get "/theme-qunit?id=#{theme.id}"
+        expect(response.status).to eq(200)
+        expect(response.body).to include("/stylesheets/color_definitions_base_")
+        expect(response.body).to include("/stylesheets/desktop_")
+        expect(response.body).to include("/stylesheets/test_helper_")
+        expect(response.body).to include("/assets/discourse/tests/theme_test_helper.js")
+        expect(response.body).to include("/assets/discourse/tests/theme_test_vendor.js")
+        expect(response.body).to match(/\/theme-javascripts\/\h{40}\.js/)
+        expect(response.body).to include("/theme-javascripts/tests/#{theme.id}-")
+        expect(response.body).to include("/assets/discourse/tests/test_starter.js")
+        expect(response.body).to include("/extra-locales/admin")
+      end
     end
   end
 end

spec/requests/theme_javascripts_controller_spec.rb

@@ -2,9 +2,19 @@
 require 'rails_helper'
 
 describe ThemeJavascriptsController do
+  include ActiveSupport::Testing::TimeHelpers
+
+  def clear_disk_cache
+    if Dir.exist?(ThemeJavascriptsController::DISK_CACHE_PATH)
+      `rm -rf #{ThemeJavascriptsController::DISK_CACHE_PATH}`
+    end
+  end
+
   let!(:theme) { Fabricate(:theme) }
   let(:theme_field) { ThemeField.create!(theme: theme, target_id: 0, name: "header", value: "<a>html</a>") }
   let(:javascript_cache) { JavascriptCache.create!(content: 'console.log("hello");', theme_field: theme_field) }
+  before { clear_disk_cache }
+  after { clear_disk_cache }
 
   describe '#show' do
     def update_digest_and_get(digest)
@@ -47,31 +57,83 @@ describe ThemeJavascriptsController do
       get "/theme-javascripts/#{javascript_cache.digest}.js"
       expect(response.status).to eq(404)
     end
-
-    def clear_disk_cache
-      if Dir.exist?(ThemeJavascriptsController::DISK_CACHE_PATH)
-        `rm #{ThemeJavascriptsController::DISK_CACHE_PATH}/*`
-      end
-    end
   end
 
   describe "#show_tests" do
-    context "theme settings" do
-      let(:component) { Fabricate(:theme, component: true, name: 'enabled-component') }
+    let(:component) { Fabricate(:theme, component: true, name: 'enabled-component') }
+    let!(:tests_field) do
+      field = component.set_field(
+        target: :tests_js,
+        type: :js,
+        name: "acceptance/some-test.js",
+        value: "assert.ok(true);"
+      )
+      component.save!
+      field
+    end
 
-      it "forces default values" do
-        ThemeField.create!(
-          theme: component,
-          target_id: Theme.targets[:settings],
-          name: "yaml",
-          value: "num_setting: 5"
-        )
-        component.reload
-        component.update_setting(:num_setting, 643)
+    before do
+      ThemeField.create!(
+        theme: component,
+        target_id: Theme.targets[:settings],
+        name: "yaml",
+        value: "num_setting: 5"
+      )
+      component.save!
+    end
 
-        get "/theme-javascripts/tests/#{component.id}.js"
-        expect(response.body).to include("require(\"discourse/lib/theme-settings-store\").registerSettings(#{component.id}, {\"num_setting\":5}, { force: true });")
-      end
+    it "forces theme settings default values" do
+      component.update_setting(:num_setting, 643)
+      _, digest = component.baked_js_tests_with_digest
+
+      get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
+      expect(response.body).to include("require(\"discourse/lib/theme-settings-store\").registerSettings(#{component.id}, {\"num_setting\":5}, { force: true });")
+      expect(response.body).to include("assert.ok(true);")
+    end
+
+    it "responds with 404 if digest is not a 40 chars hex" do
+      digest = Rack::Utils.escape('../../../../../../../../../../etc/passwd').gsub('.', '%2E')
+      get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
+      expect(response.status).to eq(404)
+
+      get "/theme-javascripts/tests/#{component.id}-abc123.js"
+      expect(response.status).to eq(404)
+    end
+
+    it "responds with 404 if theme does not exist" do
+      get "/theme-javascripts/tests/#{Theme.maximum(:id) + 1}-#{SecureRandom.hex(20)}.js"
+      expect(response.status).to eq(404)
+    end
+
+    it "responds with 304 if tests digest has not changed" do
+      content, digest = component.baked_js_tests_with_digest
+      get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
+      last_modified = Time.rfc2822(response.headers["Last-Modified"])
+      expect(response.status).to eq(200)
+      expect(response.headers["Content-Length"].to_i).to eq(content.size)
+
+      get "/theme-javascripts/tests/#{component.id}-#{digest}.js",
+        headers: { "If-Modified-Since" => (last_modified + 10.seconds).rfc2822 }
+      expect(response.status).to eq(304)
+    end
+
+    it "responds with 404 to requests with old digests" do
+      _, old_digest = component.baked_js_tests_with_digest
+      get "/theme-javascripts/tests/#{component.id}-#{old_digest}.js"
+      expect(response.status).to eq(200)
+      expect(response.body).to include("assert.ok(true);")
+
+      tests_field.update!(value: "assert.ok(343434);")
+      tests_field.invalidate_baked!
+      _, digest = component.baked_js_tests_with_digest
+      expect(old_digest).not_to eq(digest)
+
+      get "/theme-javascripts/tests/#{component.id}-#{old_digest}.js"
+      expect(response.status).to eq(404)
+
+      get "/theme-javascripts/tests/#{component.id}-#{digest}.js"
+      expect(response.status).to eq(200)
+      expect(response.body).to include("assert.ok(343434);")
     end
   end
 end