From 4dd053a69c073e7662f1718f70160b67deb4fd49 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 31 May 2023 22:00:35 +0900 Subject: [PATCH] DEV: Call `MessageBus.stop()` after each system test (#21848) ## What is the problem? MessageBus by default uses long polling which keeps a connection open for 25 seconds by default. The problem here is that Capybara does not know about these connections being kept opened by MessageBus and hence does not know how to stop these connections at the end of each test. As a result, the long polling MessageBus connections are kept opened by the browser and we hit chrome's limit of 6 concurrent requests per host, new request made in the browser is marked as "pending" until a request is freed up. Since we keep a MessageBus long polling connection opened for 25 seconds, our finders in Capybara end up hitting Capybara's wait time out causing the tests to fail. ## What is the fix? Since we can't rely on Capybara to close all the existing Capybara connections, we manually execute a script to stop all MessageBus connections after each system test. ``` for i in {1..10}; do echo "Running iteration $i" PARALLEL_TEST_PROCESSORS=8 CAPYBARA_DEFAULT_MAX_WAIT_TIME=10 bin/turbo_rspec --seed=34908 --profile --verbose --format documentation spec/system if [ $? -ne 0 ]; then echo "Error encountered on iteration $i" exit 1 fi done echo "All 10 iterations completed successfully" ``` Without the fix, the script fails consistently in the first few iterations. Running in non-headless mode with the "network" tab opened will reveal the requests that are marked as pending. --- spec/rails_helper.rb | 1 + .../admin_customize_form_templates_spec.rb | 43 ++++++++++--------- .../page_objects/pages/form_template.rb | 18 ++++++-- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 79e13579a15..69665d0bbd0 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -442,6 +442,7 @@ RSpec.configure do |config| end end + page.execute_script("if (typeof MessageBus !== 'undefined') { MessageBus.stop(); }") Capybara.reset_sessions! Capybara.use_default_driver Discourse.redis.flushdb diff --git a/spec/system/admin_customize_form_templates_spec.rb b/spec/system/admin_customize_form_templates_spec.rb index cc17e11c330..09cf3245422 100644 --- a/spec/system/admin_customize_form_templates_spec.rb +++ b/spec/system/admin_customize_form_templates_spec.rb @@ -9,10 +9,6 @@ describe "Admin Customize Form Templates", type: :system, js: true do fab!(:category) { Fabricate(:category) } before do - skip(<<~TEXT) if ENV["CI"] - The specs here are extremely flaky on CI for some reason. - TEXT - SiteSetting.experimental_form_templates = true sign_in(admin) end @@ -21,25 +17,29 @@ describe "Admin Customize Form Templates", type: :system, js: true do before { category.update!(form_template_ids: [form_template.id]) } it "should show the existing form templates in a table" do - visit("/admin/customize/form-templates") + form_template_page.visit + expect(form_template_page).to have_form_template_table expect(form_template_page).to have_form_template(form_template.name) end it "should show the categories the form template is used in" do - visit("/admin/customize/form-templates") + form_template_page.visit + expect(form_template_page).to have_form_template_table expect(form_template_page).to have_category_in_template_row(category.name) end it "should show the form template structure in a modal" do - visit("/admin/customize/form-templates") + form_template_page.visit + form_template_page.click_view_form_template expect(form_template_page).to have_template_structure("- type: input") end it "should show a preview of the template in a modal when toggling the preview" do - visit("/admin/customize/form-templates") + form_template_page.visit + form_template_page.click_view_form_template form_template_page.click_toggle_preview expect(form_template_page).to have_input_field("input") @@ -55,7 +55,7 @@ describe "Admin Customize Form Templates", type: :system, js: true do end def quick_insertion_test(field_type, content) - visit("/admin/customize/form-templates/new") + form_template_page.visit_new form_template_page.type_in_template_name("New Template") form_template_page.click_quick_insert(field_type) expect(ace_editor).to have_text(content) @@ -63,7 +63,7 @@ describe "Admin Customize Form Templates", type: :system, js: true do describe "when visiting the page to create a new form template" do it "should allow admin to create a new form template" do - visit("/admin/customize/form-templates/new") + form_template_page.visit_new sample_name = "My First Template" sample_template = "- type: input" @@ -75,40 +75,41 @@ describe "Admin Customize Form Templates", type: :system, js: true do end it "should disable the save button until form is filled out" do - visit("/admin/customize/form-templates/new") - expect(form_template_page).to have_save_button_with_state(true) + form_template_page.visit_new + expect(form_template_page).to have_save_button_with_state(disabled: true) form_template_page.type_in_template_name("New Template") - expect(form_template_page).to have_save_button_with_state(true) + expect(form_template_page).to have_save_button_with_state(disabled: true) ace_editor.type_input("- type: input") - expect(form_template_page).to have_save_button_with_state(false) + expect(form_template_page).to have_save_button_with_state(disabled: false) end it "should disable the preview button until form is filled out" do - visit("/admin/customize/form-templates/new") - expect(form_template_page).to have_preview_button_with_state(true) + form_template_page.visit_new + expect(form_template_page).to have_preview_button_with_state(disabled: true) form_template_page.type_in_template_name("New Template") - expect(form_template_page).to have_preview_button_with_state(true) + expect(form_template_page).to have_preview_button_with_state(disabled: true) ace_editor.type_input("- type: input") - expect(form_template_page).to have_preview_button_with_state(false) + expect(form_template_page).to have_preview_button_with_state(disabled: false) end it "should show validation options in a modal when clicking the validations button" do - visit("/admin/customize/form-templates/new") + form_template_page.visit_new form_template_page.click_validations_button expect(form_template_page).to have_validations_modal end it "should show a preview of the template in a modal when clicking the preview button" do - visit("/admin/customize/form-templates/new") + form_template_page.visit_new form_template_page.type_in_template_name("New Template") ace_editor.type_input("- type: input") form_template_page.click_preview_button + expect(form_template_page).to have_preview_modal expect(form_template_page).to have_input_field("input") end it "should render all the input field types in the preview" do - visit("/admin/customize/form-templates/new") + form_template_page.visit_new form_template_page.type_in_template_name("New Template") ace_editor.type_input( "- type: input\n- type: textarea\n- type: checkbox\n- type: dropdown\n- type: upload\n- type: multi-select", diff --git a/spec/system/page_objects/pages/form_template.rb b/spec/system/page_objects/pages/form_template.rb index 05e184894a5..67d4f16f26d 100644 --- a/spec/system/page_objects/pages/form_template.rb +++ b/spec/system/page_objects/pages/form_template.rb @@ -3,6 +3,16 @@ module PageObjects module Pages class FormTemplate < PageObjects::Pages::Base + def visit + page.visit("/admin/customize/form-templates") + self + end + + def visit_new + page.visit("/admin/customize/form-templates/new") + self + end + # Form Template Index def has_form_template_table? page.has_selector?("table.form-templates__table") @@ -67,12 +77,12 @@ module PageObjects find(".form-templates__form-name-input").value == name end - def has_save_button_with_state?(state) - find_button("Save", disabled: state) + def has_save_button_with_state?(disabled:) + find_button("Save", disabled:) end - def has_preview_button_with_state?(state) - find_button("Preview", disabled: state) + def has_preview_button_with_state?(disabled:) + find_button("Preview", disabled:) end end end