From 64a4390e174f4df4650136f52b7f5137eab91b32 Mon Sep 17 00:00:00 2001 From: Martin Brennan Date: Mon, 28 Aug 2023 14:59:22 +1000 Subject: [PATCH] DEV: Fix flaky network-based upload spec (#23286) Tries to fix the composer upload spec by making the upload slow enough to allow clicking the Cancel button, and improves generally the API for CDP network changes. --- .../chat/spec/system/chat_composer_spec.rb | 21 ++++----- spec/system/composer_uploads_spec.rb | 19 ++++---- spec/system/network_disconnected_spec.rb | 45 ++++++++----------- spec/system/page_objects/cdp.rb | 27 +++++++++++ spec/system/s3_uploads_spec.rb | 2 +- 5 files changed, 67 insertions(+), 47 deletions(-) diff --git a/plugins/chat/spec/system/chat_composer_spec.rb b/plugins/chat/spec/system/chat_composer_spec.rb index 06735f56832..67a99f7b0c1 100644 --- a/plugins/chat/spec/system/chat_composer_spec.rb +++ b/plugins/chat/spec/system/chat_composer_spec.rb @@ -7,6 +7,7 @@ RSpec.describe "Chat composer", type: :system do let(:chat_page) { PageObjects::Pages::Chat.new } let(:channel_page) { PageObjects::Pages::ChatChannel.new } + let(:cdp) { PageObjects::CDP.new } before do chat_system_bootstrap @@ -151,18 +152,18 @@ RSpec.describe "Chat composer", type: :system do it "doesn’t allow to send" do chat_page.visit_channel(channel_1) - page.driver.browser.network_conditions = { latency: 20_000 } - file_path = file_from_fixtures("logo.png", "images").path - attach_file(file_path) do - channel_page.open_action_menu - channel_page.click_action_button("chat-upload-btn") - end + cdp.with_slow_upload do + attach_file(file_path) do + channel_page.open_action_menu + channel_page.click_action_button("chat-upload-btn") + end - expect(page).to have_css(".chat-composer-upload--in-progress") - expect(page).to have_css(".chat-composer.is-send-disabled") - ensure - page.driver.browser.network_conditions = { latency: 0 } + expect(page).to have_css(".chat-composer-upload--in-progress") + expect(page).to have_css(".chat-composer.is-send-disabled") + page.find(".chat-composer-upload").hover + page.find(".chat-composer-upload__remove-btn").click + end end end end diff --git a/spec/system/composer_uploads_spec.rb b/spec/system/composer_uploads_spec.rb index dab2f4b4676..1b8d176fb92 100644 --- a/spec/system/composer_uploads_spec.rb +++ b/spec/system/composer_uploads_spec.rb @@ -6,6 +6,7 @@ describe "Uploading files in the composer", type: :system, js: true do let(:modal) { PageObjects::Modals::Base.new } let(:composer) { PageObjects::Components::Composer.new } let(:topic) { PageObjects::Pages::Topic.new } + let(:cdp) { PageObjects::CDP.new } before { sign_in(current_user) } @@ -29,17 +30,15 @@ describe "Uploading files in the composer", type: :system, js: true do visit "/new-topic" expect(composer).to be_opened - page.driver.browser.network_conditions = { latency: 20_000 } + file_path_1 = file_from_fixtures("huge.jpg", "images").path + cdp.with_slow_upload do + attach_file(file_path_1) { composer.click_toolbar_button("upload") } + expect(composer).to have_in_progress_uploads + find("#cancel-file-upload").click - file_path_1 = file_from_fixtures("logo.png", "images").path - attach_file(file_path_1) { composer.click_toolbar_button("upload") } - expect(composer).to have_in_progress_uploads - find("#cancel-file-upload").click - - expect(composer).to have_no_in_progress_uploads - expect(composer.preview).to have_no_css(".image-wrapper") - ensure - page.driver.browser.network_conditions = { latency: 0 } + expect(composer).to have_no_in_progress_uploads + expect(composer.preview).to have_no_css(".image-wrapper") + end end context "when video thumbnails are enabled" do diff --git a/spec/system/network_disconnected_spec.rb b/spec/system/network_disconnected_spec.rb index 7e3dd0f7041..aaeb883c894 100644 --- a/spec/system/network_disconnected_spec.rb +++ b/spec/system/network_disconnected_spec.rb @@ -1,34 +1,27 @@ # frozen_string_literal: true RSpec.describe "Network Disconnected", type: :system do - def with_network_disconnected - begin - page.driver.browser.network_conditions = { offline: true } - yield - ensure - page.driver.browser.network_conditions = { offline: false } - end - end + let(:cdp) { PageObjects::CDP.new } it "NetworkConnectivity service adds class to DOM and displays offline indicator" do - skip(<<~TEXT) - # In CI this test will randomly flake - timing issue with the offline indicator - # not being rendered soon enough after network conditions change - - SiteSetting.enable_offline_indicator = true - - visit("/c") - - expect(page).to have_no_css("html.network-disconnected") - expect(page).to have_no_css(".offline-indicator") - - with_network_disconnected doskip(<<~TEXT) - # Message bus connectivity services adds the disconnected class to the DOM - expect(page).to have_css("html.network-disconnected") - - # Offline indicator is rendered - expect(page).to have_css(".offline-indicator") - end + skip(<<~TEXT) if ENV["CI"] + In CI this test will randomly flake - timing issue with the offline indicator + not being rendered soon enough after network conditions change TEXT + + SiteSetting.enable_offline_indicator = true + + visit("/c") + + expect(page).to have_no_css("html.network-disconnected") + expect(page).to have_no_css(".offline-indicator") + + cdp.with_network_disconnected do + # Message bus connectivity services adds the disconnected class to the DOM + expect(page).to have_css("html.network-disconnected") + + # Offline indicator is rendered + expect(page).to have_css(".offline-indicator") + end end end diff --git a/spec/system/page_objects/cdp.rb b/spec/system/page_objects/cdp.rb index 56bf4922f26..8daf5a2658d 100644 --- a/spec/system/page_objects/cdp.rb +++ b/spec/system/page_objects/cdp.rb @@ -27,5 +27,32 @@ module PageObjects def read_clipboard page.evaluate_async_script("navigator.clipboard.readText().then(arguments[0])") end + + def with_network_disconnected + begin + page.driver.browser.network_conditions = { offline: true } + yield + ensure + page.driver.browser.network_conditions = { offline: false } + end + end + + def with_slow_download + begin + page.driver.browser.network_conditions = { latency: 20_000, download_throughput: 1 } + yield + ensure + page.driver.browser.network_conditions = { latency: 0 } + end + end + + def with_slow_upload + begin + page.driver.browser.network_conditions = { latency: 20_000, upload_throughput: 1 } + yield + ensure + page.driver.browser.network_conditions = { latency: 0 } + end + end end end diff --git a/spec/system/s3_uploads_spec.rb b/spec/system/s3_uploads_spec.rb index 92b1bc84e18..daec0371684 100644 --- a/spec/system/s3_uploads_spec.rb +++ b/spec/system/s3_uploads_spec.rb @@ -25,11 +25,11 @@ describe "Uploading files in the composer to S3", type: :system do end expect(page).to have_css(".avatar-uploader label[data-uploaded]") modal.click_primary_button + expect(modal).to be_closed expect(page).to have_css( "#user-avatar-uploads[data-custom-avatar-upload-id]", visible: false, ) - puts page.driver.browser.logs.get(:browser).map(&:message) expect(current_user.reload.uploaded_avatar_id).to eq( find("#user-avatar-uploads", visible: false)["data-custom-avatar-upload-id"].to_i, )