diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e76e85f20d3..f1356bf73bd 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -19,7 +19,7 @@ jobs: build: name: ${{ matrix.target }} ${{ matrix.build_type }} runs-on: ${{ (matrix.build_type == 'annotations') && 'ubuntu-latest' || 'ubuntu-20.04-8core' }} - container: discourse/discourse_test:slim${{ startsWith(matrix.build_type, 'frontend') && '-browsers' || '' }} + container: discourse/discourse_test:slim${{ (matrix.build_type == 'frontend' || matrix.build_type == 'system') && '-browsers' || '' }} timeout-minutes: 20 env: @@ -34,13 +34,15 @@ jobs: fail-fast: false matrix: - build_type: [backend, frontend, annotations] + build_type: [backend, frontend, system, annotations] target: [core, plugins] exclude: - build_type: annotations target: plugins - build_type: frontend target: core # Handled by core_frontend_tests job (below) + - build_type: system + target: plugins # Enable once at least 1 plugin has system tests steps: - uses: actions/checkout@v3 @@ -166,6 +168,25 @@ jobs: run: QUNIT_PARALLEL=3 bin/rake plugin:qunit['*','1200000'] timeout-minutes: 30 + - name: Ember Build for System Tests + if: matrix.build_type == 'system' + run: bin/ember-cli --build + + - name: Core System Tests + if: matrix.build_type == 'system' && matrix.target == 'core' + run: bin/system_rspec + + - name: Plugin System Tests + if: matrix.build_type == 'system' && matrix.target == 'plugins' + run: bin/system_rspec plugins/*/spec/system + + - name: Upload failed system test screenshots + uses: actions/upload-artifact@v3 + if: matrix.build_type == 'system' && failure() + with: + name: failed-system-test-screenshots + path: tmp/screenshots/*.png + - name: Check Annotations if: matrix.build_type == 'annotations' run: | diff --git a/Gemfile b/Gemfile index a59ecb56409..e29c6737bed 100644 --- a/Gemfile +++ b/Gemfile @@ -149,11 +149,14 @@ group :assets do end group :test do + gem 'capybara', require: false gem 'webmock', require: false gem 'fakeweb', require: false gem 'minitest', require: false gem 'simplecov', require: false + gem 'selenium-webdriver', require: false gem "test-prof" + gem 'webdrivers', require: false end group :test, :development do diff --git a/Gemfile.lock b/Gemfile.lock index 51df0fa0eb2..f1eb7ed5b8b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -89,8 +89,18 @@ GEM activesupport (>= 3.0.0) uniform_notifier (~> 1.11) byebug (11.1.3) + capybara (3.37.1) + addressable + matrix + mini_mime (>= 0.1.3) + nokogiri (~> 1.8) + rack (>= 1.6.0) + rack-test (>= 0.6.3) + regexp_parser (>= 1.5, < 3.0) + xpath (~> 3.2) cbor (0.5.9.6) certified (1.0.0) + childprocess (4.1.0) chunky_png (1.4.0) coderay (1.1.3) colored2 (3.1.2) @@ -208,6 +218,7 @@ GEM nokogiri (>= 1.5.9) lru_redux (1.1.0) lz4-ruby (0.3.3) + matrix (0.4.2) maxminddb (0.1.22) memory_profiler (1.0.0) message_bus (4.2.0) @@ -434,6 +445,11 @@ GEM seed-fu (2.3.9) activerecord (>= 3.1) activesupport (>= 3.1) + selenium-webdriver (4.4.0) + childprocess (>= 0.5, < 5.0) + rexml (~> 3.2, >= 3.2.5) + rubyzip (>= 1.2.2, < 3.0) + websocket (~> 1.0) shoulda-matchers (5.2.0) activesupport (>= 5.2.0) sidekiq (6.5.7) @@ -478,6 +494,10 @@ GEM uri (0.11.0) uri_template (0.7.0) version_gem (1.1.0) + webdrivers (5.1.0) + nokogiri (~> 1.6) + rubyzip (>= 1.3.0) + selenium-webdriver (~> 4.0) webmock (3.18.1) addressable (>= 2.8.0) crack (>= 0.3.2) @@ -485,7 +505,10 @@ GEM webpush (1.1.0) hkdf (~> 0.2) jwt (~> 2.0) + websocket (1.2.9) xorcist (1.1.3) + xpath (3.2.0) + nokogiri (~> 1.8) yaml-lint (0.0.10) zeitwerk (2.6.0) @@ -517,6 +540,7 @@ DEPENDENCIES bootsnap bullet byebug + capybara cbor certified colored2 @@ -609,6 +633,7 @@ DEPENDENCIES sassc (= 2.0.1) sassc-rails seed-fu + selenium-webdriver shoulda-matchers sidekiq simplecov @@ -621,6 +646,7 @@ DEPENDENCIES uglifier unf unicorn + webdrivers webmock webpush xorcist diff --git a/bin/ember-cli b/bin/ember-cli index 7f5decb36fc..d1c480085cc 100755 --- a/bin/ember-cli +++ b/bin/ember-cli @@ -7,7 +7,7 @@ RAILS_ROOT = File.expand_path("../../", Pathname.new(__FILE__).realpath) PORT = ENV["UNICORN_PORT"] ||= "3000" HOSTNAME = ENV["DISCOURSE_HOSTNAME"] ||= "127.0.0.1" YARN_DIR = File.join(RAILS_ROOT, "app/assets/javascripts/discourse") -CUSTOM_ARGS = ["--try", "--test", "--unicorn", "-u", "--forward-host"] +CUSTOM_ARGS = ["--try", "--test", "--build", "--unicorn", "-u", "--forward-host"] PROXY = if ARGV.include?("--try") "https://try.discourse.org" @@ -24,6 +24,8 @@ end command = if ARGV.include?("--test") "test" + elsif ARGV.include?("--build") + "build" else "server" end @@ -49,7 +51,7 @@ end args = ["-s", "--cwd", YARN_DIR, "run", "ember", command] + (ARGV - CUSTOM_ARGS) -if !args.include?("test") && !args.include?("--proxy") +if !args.include?("test") && !args.include?("build") && !args.include?("--proxy") args << "--proxy" args << PROXY end diff --git a/bin/system_rspec b/bin/system_rspec new file mode 100755 index 00000000000..3f9e0f2adf3 --- /dev/null +++ b/bin/system_rspec @@ -0,0 +1,7 @@ +#!/bin/bash + +if [[ -z "$@" ]]; then + bin/rspec spec/system +elif [[ -n "$@" ]]; then + bin/rspec "$@" +fi diff --git a/bin/turbo_rspec b/bin/turbo_rspec index 66464bc67b5..7d4c4278163 100755 --- a/bin/turbo_rspec +++ b/bin/turbo_rspec @@ -59,10 +59,18 @@ formatters.each do |formatter| end end +# We do not want to include system specs by default, they are super +# slow and require a selenium server to be running. +default_spec_dirs = Dir.entries("#{Rails.root}/spec").reject do |entry| + !File.directory?("spec/#{entry}") || ["..", ".", "system"].include?(entry) +end.map { |entry| "spec/#{entry}" } +files = ARGV.empty? ? default_spec_dirs : ARGV + +puts "Running turbo_rspec using files in #{files}" success = TurboTests::Runner.run( formatters: formatters, - files: ARGV.empty? ? ["spec"] : ARGV, + files: files, verbose: verbose, fail_fast: fail_fast ) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 1704c0e23b0..e825618a942 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -57,6 +57,9 @@ require 'shoulda-matchers' require 'sidekiq/testing' require 'test_prof/recipes/rspec/let_it_be' require 'test_prof/before_all/adapters/active_record' +require 'webdrivers' +require 'selenium-webdriver' +require 'capybara/rails' # The shoulda-matchers gem no longer detects the test framework # you're using or mixes itself into that framework automatically. @@ -71,6 +74,11 @@ end # Requires supporting ruby files with custom matchers and macros, etc, # in spec/support/ and its subdirectories. Dir[Rails.root.join("spec/support/**/*.rb")].each { |f| require f } + +require Rails.root.join("spec/system/page_objects/pages/base.rb") +require Rails.root.join("spec/system/page_objects/modals/base.rb") +Dir[Rails.root.join("spec/system/page_objects/**/*.rb")].each { |f| require f } + Dir[Rails.root.join("spec/fabricators/*.rb")].each { |f| require f } require_relative './helpers/redis_snapshot_helper' @@ -83,6 +91,10 @@ if ENV['LOAD_PLUGINS'] == "1" Dir[Rails.root.join("plugins/*/spec/fabricators/**/*.rb")].each do |f| require f end + + Dir[Rails.root.join("plugins/*/spec/system/page_objects/**/*.rb")].each do |f| + require f + end end # let's not run seed_fu every test @@ -177,6 +189,7 @@ RSpec.configure do |config| config.include MessageBus config.include RSpecHtmlMatchers config.include IntegrationHelpers, type: :request + config.include SystemHelpers, type: :system config.include WebauthnIntegrationHelpers config.include SiteSettingsHelpers config.include SidekiqHelpers @@ -228,7 +241,41 @@ RSpec.configure do |config| SiteSetting.provider = TestLocalProcessProvider.new - WebMock.disable_net_connect! + WebMock.disable_net_connect!( + allow_localhost: true, + allow: [Webdrivers::Chromedriver.base_url] + ) + + Capybara.configure do |capybara_config| + capybara_config.server_host = "localhost" + capybara_config.server_port = 31337 + end + + chrome_browser_options = Selenium::WebDriver::Chrome::Options.new( + logging_prefs: { "browser" => "ALL", "driver" => "ALL" } + ).tap do |options| + options.add_argument("--window-size=1400,1400") + options.add_argument("--no-sandbox") + options.add_argument("--disable-dev-shm-usage") + end + + Capybara.register_driver :selenium_chrome do |app| + Capybara::Selenium::Driver.new( + app, + browser: :chrome, + capabilities: chrome_browser_options, + ) + end + + Capybara.register_driver :selenium_chrome_headless do |app| + chrome_browser_options.add_argument("--headless") + + Capybara::Selenium::Driver.new( + app, + browser: :chrome, + capabilities: chrome_browser_options, + ) + end if ENV['ELEVATED_UPLOADS_ID'] DB.exec "SELECT setval('uploads_id_seq', 10000)" @@ -246,14 +293,14 @@ RSpec.configure do |config| end end - config.after :each do |x| - if x.exception && ex = RspecErrorTracker.last_exception + config.after :each do |example| + if example.exception && ex = RspecErrorTracker.last_exception # magic in a cause if we have none - unless x.exception.cause - class << x.exception + unless example.exception.cause + class << example.exception attr_accessor :cause end - x.exception.cause = ex + example.exception.cause = ex end end @@ -286,8 +333,53 @@ RSpec.configure do |config| end # Match the request hostname to the value in `database.yml` - config.before(:all, type: [:request, :multisite]) { host! "test.localhost" } - config.before(:each, type: [:request, :multisite]) { host! "test.localhost" } + config.before(:all, type: [:request, :multisite, :system]) { host! "test.localhost" } + config.before(:each, type: [:request, :multisite, :system]) { host! "test.localhost" } + + last_driven_by = nil + config.before(:each, type: :system) do |example| + if example.metadata[:js] + driver = "selenium_chrome" + driver += "_headless" unless ENV["SELENIUM_HEADLESS"] == "0" + driven_by driver.to_sym + end + setup_system_test + end + + config.after(:each, type: :system) do |example| + # This is disabled by default because it is super verbose, + # if you really need to dig into how selenium is communicating + # for system tests then enable it. + if ENV["SELENIUM_VERBOSE_DRIVER_LOGS"] + puts "~~~~~~ DRIVER LOGS: ~~~~~~~" + page.driver.browser.logs.get(:driver).each do |log| + puts log.message + end + end + + # Recommended that this is not disabled, since it makes debugging + # failed system tests a lot trickier. + if ENV["SELENIUM_DISABLE_VERBOSE_JS_LOGS"].blank? + if example.exception + skip_js_errors = false + if example.exception.kind_of?(RSpec::Core::MultipleExceptionError) + puts "~~~~~~ SYSTEM TEST ERRORS: ~~~~~~~" + example.exception.all_exceptions.each do |ex| + puts ex.message + end + + skip_js_errors = true + end + + if !skip_js_errors + puts "~~~~~~ JS ERRORS: ~~~~~~~" + page.driver.browser.logs.get(:browser).each do |log| + puts log.message + end + end + end + end + end config.before(:each, type: :multisite) do Rails.configuration.multisite = true # rubocop:disable Discourse/NoDirectMultisiteManipulation diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb new file mode 100644 index 00000000000..b7116b2a4c4 --- /dev/null +++ b/spec/support/system_helpers.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module SystemHelpers + def sign_in(user) + visit "/session/#{user.encoded_username}/become" + end + + def setup_system_test + SiteSetting.login_required = false + SiteSetting.content_security_policy = false + SiteSetting.force_hostname = "#{Capybara.server_host}:#{Capybara.server_port}" + SiteSetting.external_system_avatars_enabled = false + end + + def try_until_success(timeout: 2, frequency: 0.01) + start ||= Time.zone.now + backoff ||= frequency + yield + rescue RSpec::Expectations::ExpectationNotMetError + if Time.zone.now >= start + timeout.seconds + raise + end + sleep backoff + backoff += frequency + retry + end +end diff --git a/spec/system/bookmarks_spec.rb b/spec/system/bookmarks_spec.rb new file mode 100644 index 00000000000..82802505a92 --- /dev/null +++ b/spec/system/bookmarks_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +describe "Bookmarking posts and topics", type: :system, js: true do + fab!(:topic) { Fabricate(:topic) } + fab!(:user) { Fabricate(:user, username: "bookmarkguy") } + fab!(:post) { Fabricate(:post, topic: topic, raw: "This is some post to bookmark") } + fab!(:post2) { Fabricate(:post, topic: topic, raw: "Some interesting post content") } + + it "allows logged in user to create bookmarks with and without reminders" do + sign_in user + visit "/t/#{topic.id}" + topic_page = PageObjects::Pages::Topic.new + expect(topic_page).to have_post_content(post) + topic_page.expand_post_actions(post) + topic_page.click_post_action_button(post, :bookmark) + + bookmark_modal = PageObjects::Modals::Bookmark.new + bookmark_modal.fill_name("something important") + bookmark_modal.save + + expect(topic_page).to have_post_bookmarked(post) + bookmark = Bookmark.find_by(bookmarkable: post, user: user) + expect(bookmark.name).to eq("something important") + + topic_page.expand_post_actions(post2) + topic_page.click_post_action_button(post2, :bookmark) + + bookmark_modal = PageObjects::Modals::Bookmark.new + bookmark_modal.select_preset_reminder(:tomorrow) + expect(topic_page).to have_post_bookmarked(post2) + bookmark = Bookmark.find_by(bookmarkable: post2, user: user) + expect(bookmark.reminder_at).not_to eq(nil) + expect(bookmark.reminder_set_at).not_to eq(nil) + end + + it "does not create a bookmark if the modal is closed with the cancel button" do + sign_in user + visit "/t/#{topic.id}" + topic_page = PageObjects::Pages::Topic.new + topic_page.expand_post_actions(post) + topic_page.click_post_action_button(post, :bookmark) + + bookmark_modal = PageObjects::Modals::Bookmark.new + bookmark_modal.fill_name("something important") + bookmark_modal.cancel + + expect(topic_page).not_to have_post_bookmarked(post) + expect(Bookmark.exists?(bookmarkable: post, user: user)).to eq(false) + end + + it "allows the topic to be bookmarked" do + sign_in user + visit "/t/#{topic.id}" + topic_page = PageObjects::Pages::Topic.new + topic_page.click_topic_footer_button(:bookmark) + + bookmark_modal = PageObjects::Modals::Bookmark.new + bookmark_modal.fill_name("something important") + bookmark_modal.save + + expect(topic_page).to have_topic_bookmarked + bookmark = try_until_success do + expect(Bookmark.exists?(bookmarkable: topic, user: user)).to eq(true) + end + expect(bookmark).not_to eq(nil) + end +end diff --git a/spec/system/page_objects/modals/base.rb b/spec/system/page_objects/modals/base.rb new file mode 100644 index 00000000000..1e7f477a859 --- /dev/null +++ b/spec/system/page_objects/modals/base.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class Base + include Capybara::DSL + include RSpec::Matchers + + def close + find(".modal-close").click + end + + def cancel + find(".d-modal-cancel").click + end + end + end +end diff --git a/spec/system/page_objects/modals/bookmark.rb b/spec/system/page_objects/modals/bookmark.rb new file mode 100644 index 00000000000..c9d3ac6c946 --- /dev/null +++ b/spec/system/page_objects/modals/bookmark.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module PageObjects + module Modals + class Bookmark < PageObjects::Modals::Base + def fill_name(name) + fill_in "bookmark-name", with: name + end + + def select_preset_reminder(identifier) + find("#tap_tile_#{identifier}").click + end + + def save + find("#save-bookmark").click + end + end + end +end diff --git a/spec/system/page_objects/pages/base.rb b/spec/system/page_objects/pages/base.rb new file mode 100644 index 00000000000..60c2ffb24f2 --- /dev/null +++ b/spec/system/page_objects/pages/base.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class Base + include Capybara::DSL + + def setup_component_classes!(component_classes) + @component_classes = component_classes + end + end + end +end diff --git a/spec/system/page_objects/pages/topic.rb b/spec/system/page_objects/pages/topic.rb new file mode 100644 index 00000000000..aff0dc4ccfb --- /dev/null +++ b/spec/system/page_objects/pages/topic.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module PageObjects + module Pages + class Topic < PageObjects::Pages::Base + def initialize + setup_component_classes!( + post_show_more_actions: ".show-more-actions", + post_action_button_bookmark: ".bookmark.with-reminder" + ) + end + + def has_post_content?(post) + post_by_number(post).has_content? post.raw + end + + def has_post_more_actions?(post) + within post_by_number(post) do + has_css?(@component_classes[:post_show_more_actions]) + end + end + + def has_post_bookmarked?(post) + within post_by_number(post) do + has_css?(@component_classes[:post_action_button_bookmark] + ".bookmarked") + end + end + + def expand_post_actions(post) + post_by_number(post).find(@component_classes[:post_show_more_actions]).click + end + + def click_post_action_button(post, button) + post_by_number(post).find(@component_classes["post_action_button_#{button}".to_sym]).click + end + + def click_topic_footer_button(button) + find_topic_footer_button(button).click + end + + def has_topic_bookmarked? + has_css?("#{topic_footer_button_id("bookmark")}.bookmarked", text: "Edit Bookmark") + end + + def find_topic_footer_button(button) + find(topic_footer_button_id(button)) + end + + private + + def topic_footer_button_id(button) + "#topic-footer-button-#{button}" + end + + def post_by_number(post) + find("#post_#{post.post_number}") + end + end + end +end