diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e5c3b6b9058..a5bd859ea8d 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -48,8 +48,6 @@ jobs: target: plugins - build_type: annotations target: themes - - build_type: frontend - target: themes - build_type: backend target: themes - build_type: frontend @@ -120,7 +118,7 @@ jobs: run: bin/rake plugin:pull_compatible_all - name: Checkout official themes - if: matrix.target == 'themes' + if: matrix.target == 'themes' && matrix.build_type == 'system' run: bin/rake themes:clone_all_official - name: Add hosts to /etc/hosts, otherwise Chrome cannot reach minio @@ -222,6 +220,11 @@ jobs: run: QUNIT_WRITE_EXECUTION_FILE=1 QUNIT_PARALLEL=3 bin/rake plugin:qunit['*','1200000'] timeout-minutes: 30 + - name: Theme QUnit + if: matrix.build_type == 'frontend' && matrix.target == 'themes' + run: DISCOURSE_DEV_DB=discourse_test QUNIT_PARALLEL=3 bin/rake themes:qunit_all_official + timeout-minutes: 15 + - uses: actions/upload-artifact@v3 if: always() && matrix.build_type == 'frontend' && matrix.target == 'plugins' with: diff --git a/app/assets/javascripts/discourse/testem.js b/app/assets/javascripts/discourse/testem.js index a392ea6c0ae..6329c325238 100644 --- a/app/assets/javascripts/discourse/testem.js +++ b/app/assets/javascripts/discourse/testem.js @@ -156,17 +156,21 @@ if (process.env.TESTEM_FIREFOX_PATH) { } const target = `http://127.0.0.1:${process.env.UNICORN_PORT || "3000"}`; +const themeTestPages = process.env.THEME_TEST_PAGES; -if (process.argv.includes("-t")) { - // Running testem without ember cli. Probably for theme-qunit - const testPage = process.argv[process.argv.indexOf("-t") + 1]; - +if (themeTestPages) { + module.exports.test_page = themeTestPages.split(","); module.exports.proxies = {}; + + // Prepend a prefix to the path of the route such that the server handling the request can easily identify `/theme-qunit` + // requests. This is required because testem prepends a string to the path of the `test_page` option when it makes + // the request and there is no easy way for us to strip the string from the path through the proxy. As such, we let the + // destination server handle the request base on the prefix instead. module.exports.proxies[`/*/theme-qunit`] = { - target: `${target}${testPage}`, - ignorePath: true, + target: `${target}/testem-theme-qunit`, xfwd: true, }; + module.exports.proxies["/*/*"] = { target, xfwd: true }; module.exports.middleware = [ diff --git a/app/models/remote_theme.rb b/app/models/remote_theme.rb index 32db3d99ca4..85f7f47ed28 100644 --- a/app/models/remote_theme.rb +++ b/app/models/remote_theme.rb @@ -80,8 +80,8 @@ class RemoteTheme < ActiveRecord::Base ) end - # This is only used in the tests environment and is currently not supported for other environments - if Rails.env.test? + # This is only used in the development and test environment and is currently not supported for other environments + if Rails.env.test? || Rails.env.development? def self.import_theme_from_directory(directory) update_theme(ThemeStore::DirectoryImporter.new(directory)) end diff --git a/config/initializers/006-mini_profiler.rb b/config/initializers/006-mini_profiler.rb index 3ea4a6fa798..5fd7b109258 100644 --- a/config/initializers/006-mini_profiler.rb +++ b/config/initializers/006-mini_profiler.rb @@ -54,8 +54,7 @@ if defined?(Rack::MiniProfiler) && defined?(Rack::MiniProfiler::Config) /topics/timings /uploads/ /user_avatar/ - /theme-qunit - ].map { |path| "#{Discourse.base_path}#{path}" } + ].map { |path| "#{Discourse.base_path}#{path}" }.concat([/.*theme-qunit/]) # we DO NOT WANT mini-profiler loading on anything but real desktops and laptops # so let's rule out all handheld, tablet, and mobile devices diff --git a/config/routes.rb b/config/routes.rb index 56ddd26069e..165c71a8356 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1583,6 +1583,16 @@ Discourse::Application.routes.draw do get "/theme-qunit" => "qunit#theme" + # This is a special route that is used when theme QUnit tests are run through testem which appends a testem_id to the + # path. Unfortunately, testem's proxy support does not allow us to easily remove this from the path, so we have to + # handle it here. + if Rails.env.development? + get "/testem-theme-qunit/:testem_id/theme-qunit" => "qunit#theme", + :constraints => { + testem_id: /\d+/, + } + end + post "/push_notifications/subscribe" => "push_notification#subscribe" post "/push_notifications/unsubscribe" => "push_notification#unsubscribe" diff --git a/lib/tasks/qunit.rake b/lib/tasks/qunit.rake index 91ed4f86765..51eb704b140 100644 --- a/lib/tasks/qunit.rake +++ b/lib/tasks/qunit.rake @@ -100,24 +100,37 @@ task "qunit:test", %i[timeout qunit_path filter] do |_, args| end puts "Rails server is warmed up" - cmd = ["env", "UNICORN_PORT=#{unicorn_port}"] + env = { "UNICORN_PORT" => unicorn_port.to_s } + cmd = [] + + parallel = ENV["QUNIT_PARALLEL"] if qunit_path # Bypass `ember test` - it only works properly for the `/tests` path. # We have to trigger a `build` manually so that JS is available for rails to serve. system("yarn", "ember", "build", chdir: "#{Rails.root}/app/assets/javascripts/discourse") - test_page = "#{qunit_path}?#{query}&testem=1" - cmd += ["yarn", "testem", "ci", "-f", "testem.js", "-t", test_page] + + env["THEME_TEST_PAGES"] = if ENV["THEME_IDS"] + ENV["THEME_IDS"] + .split("|") + .map { |theme_id| "#{qunit_path}?#{query}&testem=1&id=#{theme_id}" } + .join(",") + else + "#{qunit_path}?#{query}&testem=1" + end + + cmd += %w[yarn testem ci -f testem.js] + cmd += ["--parallel", parallel] if parallel else cmd += ["yarn", "ember", "exam", "--query", query] - if parallel = ENV["QUNIT_PARALLEL"] - cmd += ["--load-balance", "--parallel", parallel] - end + cmd += ["--load-balance", "--parallel", parallel] if parallel cmd += ["--filter", filter] if filter cmd << "--write-execution-file" if ENV["QUNIT_WRITE_EXECUTION_FILE"] end - system(*cmd, chdir: "#{Rails.root}/app/assets/javascripts/discourse") + # Print out all env for debugging purposes + p env + system(env, *cmd, chdir: "#{Rails.root}/app/assets/javascripts/discourse") success &&= $?.success? ensure diff --git a/lib/tasks/themes.rake b/lib/tasks/themes.rake index 58c7b5d93be..e2dc57c762b 100644 --- a/lib/tasks/themes.rake +++ b/lib/tasks/themes.rake @@ -124,7 +124,7 @@ desc "Run QUnit tests of a theme/component" task "themes:qunit", :type, :value do |t, args| type = args[:type] value = args[:value] - raise <<~TEXT if !%w[name url id].include?(type) || value.blank? + raise <<~TEXT if !%w[name url id ids].include?(type) || value.blank? Wrong arguments type:#{type.inspect}, value:#{value.inspect}" Usage: `bundle exec rake "themes:qunit[url,]"` @@ -132,7 +132,10 @@ task "themes:qunit", :type, :value do |t, args| `bundle exec rake "themes:qunit[name,]"` OR `bundle exec rake "themes:qunit[id,]"` + OR + `bundle exec rake "themes:qunit[ids,] TEXT + ENV["THEME_#{type.upcase}"] = value.to_s ENV["QUNIT_RAILS_ENV"] ||= "development" # qunit:test will switch to `test` by default Rake::Task["qunit:test"].reenable @@ -195,7 +198,7 @@ ensure redis&.remove end -desc "Clones all official themes." +desc "Clones all official themes" task "themes:clone_all_official" do |task, args| require "theme_metadata" FileUtils.rm_rf("tmp/themes") @@ -203,7 +206,7 @@ task "themes:clone_all_official" do |task, args| official_themes = ThemeMetadata::OFFICIAL_THEMES.each do |theme_name| repo = "https://github.com/discourse/#{theme_name}" - path = File.expand_path("tmp/themes/#{theme_name}") + path = File.join(Rails.root, "tmp/themes/#{theme_name}") attempts = 0 @@ -217,3 +220,24 @@ task "themes:clone_all_official" do |task, args| end end end + +# Note that this should only be used in CI where it is safe to mutate the database without rolling back since running +# the themes QUnit tests requires the themes to be installed in the database. +desc "Runs qunit tests for all official themes" +task "themes:qunit_all_official" => ["themes:clone_all_official", :environment] do |task, args| + theme_ids_with_qunit_tests = [] + + ThemeMetadata::OFFICIAL_THEMES.each do |theme_name| + path = File.join(Rails.root, "tmp/themes/#{theme_name}") + + if Dir.glob("#{File.join(path, "test")}/**/*.{js,es6}").any? + theme = RemoteTheme.import_theme_from_directory(path) + theme_ids_with_qunit_tests << theme.id + else + puts "Skipping #{theme_name} as no QUnit tests have been detected" + end + end + + Rake::Task["themes:qunit"].reenable + Rake::Task["themes:qunit"].invoke("ids", theme_ids_with_qunit_tests.join("|")) +end