From 68a3f7783e76064feb19cf86de53f3f4367e80d1 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 20 Dec 2023 13:20:14 +0800 Subject: [PATCH] DEV: Remove the use of `Capybara::Session#quit` (#24978) Why this change? This is what `Capybara::Session#quit` does: ``` def quit @driver.quit if @driver.respond_to? :quit @document = @driver = nil @touched = false @server&.reset_error! end ``` One notable thing is that it resets server errors which means that any server errors encountered by a session is cleared. That is not what we want since it hides errors even though `Capybara.raise_server_errors` has been set to `true`. --- plugins/chat/spec/system/chat_channel_spec.rb | 8 ++------ .../chat/spec/system/dates_separators_spec.rb | 3 +-- plugins/chat/spec/system/deleted_message_spec.rb | 4 ++-- plugins/chat/spec/system/edited_message_spec.rb | 3 +-- .../chat/spec/system/react_to_message_spec.rb | 4 +--- .../spec/system/reply_to_message/smoke_spec.rb | 8 ++------ plugins/chat/spec/system/restore_message_spec.rb | 12 ++++-------- plugins/chat/spec/system/send_message_spec.rb | 16 ++++------------ .../spec/system/thread_list/full_page_spec.rb | 3 +-- .../chat/spec/system/unfollow_dm_channel_spec.rb | 3 +-- .../user_menu_notifications/sidebar_spec.rb | 3 +-- spec/support/system_helpers.rb | 3 +-- 12 files changed, 21 insertions(+), 49 deletions(-) diff --git a/plugins/chat/spec/system/chat_channel_spec.rb b/plugins/chat/spec/system/chat_channel_spec.rb index 52b581bf7d7..adf5c0673fc 100644 --- a/plugins/chat/spec/system/chat_channel_spec.rb +++ b/plugins/chat/spec/system/chat_channel_spec.rb @@ -104,14 +104,10 @@ RSpec.describe "Chat channel", type: :system do chat_page.visit_channel(channel_1) end - using_session(:tab_1) do |session| - channel_page.send_message("test_message") - session.quit - end + using_session(:tab_1) { channel_page.send_message("test_message") } - using_session(:tab_2) do |session| + using_session(:tab_2) do expect(channel_page.messages).to have_message(text: "test_message") - session.quit end end end diff --git a/plugins/chat/spec/system/dates_separators_spec.rb b/plugins/chat/spec/system/dates_separators_spec.rb index b72f63fe079..e75a9c23541 100644 --- a/plugins/chat/spec/system/dates_separators_spec.rb +++ b/plugins/chat/spec/system/dates_separators_spec.rb @@ -48,11 +48,10 @@ RSpec.describe "Dates separators", type: :system do channel_page.send_message("message1") chat_page.visit_channel(channel_2) - using_session(:user_1) do |session| + using_session(:user_1) do sign_in(user_1) chat_page.visit_channel(channel_1) channel_page.send_message("message2") - session.quit end chat_page.visit_channel(channel_1) diff --git a/plugins/chat/spec/system/deleted_message_spec.rb b/plugins/chat/spec/system/deleted_message_spec.rb index 62427b251a2..ffce295b9f7 100644 --- a/plugins/chat/spec/system/deleted_message_spec.rb +++ b/plugins/chat/spec/system/deleted_message_spec.rb @@ -59,11 +59,11 @@ RSpec.describe "Deleted message", type: :system do other_user = Fabricate(:admin) chat_system_user_bootstrap(user: other_user, channel: channel_1) - using_session(:tab_2) do |session| + + using_session(:tab_2) do sign_in(other_user) chat_page.visit_channel(channel_1) channel_page.messages.delete(message) - session.quit end sidebar_component.click_link(channel_1.name) diff --git a/plugins/chat/spec/system/edited_message_spec.rb b/plugins/chat/spec/system/edited_message_spec.rb index 7d3264de2e0..8a15c4c1490 100644 --- a/plugins/chat/spec/system/edited_message_spec.rb +++ b/plugins/chat/spec/system/edited_message_spec.rb @@ -23,12 +23,11 @@ RSpec.describe "Edited message", type: :system do it "shows as edited for all users" do chat_page.visit_channel(channel_1) - using_session(:user_1) do |session| + using_session(:user_1) do sign_in(editing_user) chat_page.visit_channel(channel_1) channel_page.edit_message(message_1, "a different message") expect(page).to have_content(I18n.t("js.chat.edited")) - session.quit end expect(page).to have_content(I18n.t("js.chat.edited")) diff --git a/plugins/chat/spec/system/react_to_message_spec.rb b/plugins/chat/spec/system/react_to_message_spec.rb index 0fd6c48d436..c0f27c016ee 100644 --- a/plugins/chat/spec/system/react_to_message_spec.rb +++ b/plugins/chat/spec/system/react_to_message_spec.rb @@ -81,14 +81,12 @@ RSpec.describe "React to message", type: :system do chat.visit_channel(category_channel_1) end - using_session(:tab_1) do |session| + using_session(:tab_1) do channel.hover_message(message_1) find(".chat-message-react-btn").click find(".chat-emoji-picker [data-emoji=\"#{reaction}\"]").click expect(channel).to have_reaction(message_1, reaction) - - session.quit end expect(channel).to have_reaction(message_1, "grimacing") diff --git a/plugins/chat/spec/system/reply_to_message/smoke_spec.rb b/plugins/chat/spec/system/reply_to_message/smoke_spec.rb index 26545f6377f..70b88b91168 100644 --- a/plugins/chat/spec/system/reply_to_message/smoke_spec.rb +++ b/plugins/chat/spec/system/reply_to_message/smoke_spec.rb @@ -40,7 +40,7 @@ RSpec.describe "Reply to message - smoke", type: :system do expect(thread_page.messages).to have_message(text: "user1reply") end - using_session(:user_2) do |session| + using_session(:user_2) do expect(thread_page.messages).to have_message(text: "user1reply") expect(channel_page.message_thread_indicator(original_message)).to have_reply_count(1) @@ -49,16 +49,12 @@ RSpec.describe "Reply to message - smoke", type: :system do expect(thread_page.messages).to have_message(text: "user1reply") expect(thread_page.messages).to have_message(text: "user2reply") expect(channel_page.message_thread_indicator(original_message)).to have_reply_count(2) - - session.quit end - using_session(:user_1) do |session| + using_session(:user_1) do expect(thread_page.messages).to have_message(text: "user1reply") expect(thread_page.messages).to have_message(text: "user2reply") expect(channel_page.message_thread_indicator(original_message)).to have_reply_count(2) - - session.quit end end end diff --git a/plugins/chat/spec/system/restore_message_spec.rb b/plugins/chat/spec/system/restore_message_spec.rb index ede9b5e0748..db8fc10ab77 100644 --- a/plugins/chat/spec/system/restore_message_spec.rb +++ b/plugins/chat/spec/system/restore_message_spec.rb @@ -35,16 +35,14 @@ RSpec.describe "Restore message", type: :system do chat_page.visit_channel(channel_1) end - using_session(:regular_user) do |session| + using_session(:regular_user) do sign_in(regular_user) chat_page.visit_channel(channel_1) channel_page.messages.delete(message_1) - session.quit end - using_session(:another_user) do |session| + using_session(:another_user) do expect(channel_page.messages).to have_no_message(id: message_1.id) - session.quit end end end @@ -58,18 +56,16 @@ RSpec.describe "Restore message", type: :system do chat_page.visit_channel(channel_1) end - using_session(:admin_user) do |session| + using_session(:admin_user) do sign_in(admin_user) chat_page.visit_channel(channel_1) channel_page.messages.delete(message_1) - session.quit end - using_session(:regular_user) do |session| + using_session(:regular_user) do expect(channel_page.messages).to have_deleted_message(message_1, count: 1) channel_page.messages.expand(id: message_1.id) expect(channel_page.messages).to have_no_action("restore", id: message_1.id) - session.quit end end end diff --git a/plugins/chat/spec/system/send_message_spec.rb b/plugins/chat/spec/system/send_message_spec.rb index 05e49104346..52064499169 100644 --- a/plugins/chat/spec/system/send_message_spec.rb +++ b/plugins/chat/spec/system/send_message_spec.rb @@ -50,18 +50,14 @@ RSpec.describe "Send message", type: :system do expect(chat_page.sidebar).to have_no_direct_message_channel(channel_1) end - using_session(:user_1) do |session| + using_session(:user_1) do channel_page.send_message expect(chat_page.sidebar).to have_direct_message_channel(channel_1) - - session.quit end - using_session(:user_2) do |session| + using_session(:user_2) do expect(chat_page.sidebar).to have_direct_message_channel(channel_1, mention: true) - - session.quit end end end @@ -96,18 +92,14 @@ RSpec.describe "Send message", type: :system do expect(chat_page.sidebar).to have_direct_message_channel(channel_1) end - using_session(:user_1) do |session| + using_session(:user_1) do channel_page.send_message expect(chat_page.sidebar).to have_direct_message_channel(channel_1) - - session.quit end - using_session(:user_2) do |session| + using_session(:user_2) do expect(chat_page.sidebar).to have_direct_message_channel(channel_1, mention: true) - - session.quit end end end diff --git a/plugins/chat/spec/system/thread_list/full_page_spec.rb b/plugins/chat/spec/system/thread_list/full_page_spec.rb index 69a0f4f2ac7..6ed0bc2e1c2 100644 --- a/plugins/chat/spec/system/thread_list/full_page_spec.rb +++ b/plugins/chat/spec/system/thread_list/full_page_spec.rb @@ -172,7 +172,7 @@ describe "Thread list in side panel | full page", type: :system do expect(thread_list_page).to have_no_thread(thread_1) - using_session(:tab_2) do |session| + using_session(:tab_2) do sign_in(other_user) chat_page.visit_channel(channel) expect(channel_page).to have_no_loading_skeleton @@ -180,7 +180,6 @@ describe "Thread list in side panel | full page", type: :system do channel_page.message_thread_indicator(thread_1.original_message).click expect(side_panel_page).to have_open_thread(thread_1) thread_page.messages.restore(thread_1.original_message) - session.quit end expect(thread_list_page).to have_thread(thread_1) diff --git a/plugins/chat/spec/system/unfollow_dm_channel_spec.rb b/plugins/chat/spec/system/unfollow_dm_channel_spec.rb index 3d5a9d12b81..c941c14ea7d 100644 --- a/plugins/chat/spec/system/unfollow_dm_channel_spec.rb +++ b/plugins/chat/spec/system/unfollow_dm_channel_spec.rb @@ -22,13 +22,12 @@ RSpec.describe "Unfollow dm channel", type: :system do expect(page).to have_no_css(".channel-#{dm_channel_1.id}") - using_session(:user_1) do |session| + using_session(:user_1) do text = "this is fine" sign_in(other_user) chat_page.visit_channel(dm_channel_1) channel_page.send_message(text) expect(channel_page.messages).to have_message(text: text) - session.quit end expect(page).to have_css(".channel-#{dm_channel_1.id} .urgent", wait: 25) diff --git a/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb b/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb index 158e284ab12..58ad3cde5b5 100644 --- a/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb +++ b/plugins/chat/spec/system/user_menu_notifications/sidebar_spec.rb @@ -213,14 +213,13 @@ RSpec.describe "User menu notifications | sidebar", type: :system do channel.send_message("this is fine @#{other_user.username}") find(".invite-link", wait: 5).click - using_session(:user_1) do |session| + using_session(:user_1) do sign_in(other_user) visit("/") find(".header-dropdown-toggle.current-user").click expect(find("#user-menu-button-chat-notifications")).to have_content(1) expect(find("#quick-access-all-notifications")).to have_css(".chat-invitation.unread") - session.quit end end end diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index f7db3707d4d..e230e0f1810 100644 --- a/spec/support/system_helpers.rb +++ b/spec/support/system_helpers.rb @@ -148,10 +148,9 @@ module SystemHelpers end def using_browser_timezone(timezone, &example) - using_session(timezone) do |session| + using_session(timezone) do page.driver.browser.devtools.emulation.set_timezone_override(timezone_id: timezone) freeze_time(&example) - session.quit end end