From c67f75f2db590af875c09dc1824d46d460e37297 Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 3 Dec 2024 16:58:25 +0100 Subject: [PATCH 1/2] test: Wait for modal to open before testing its content Capybara needs to be told to expect that the modal is open before we can check for content present in it. Fixes a lot of extremely flaky specs. --- admin/spec/features/adjustment_reasons_spec.rb | 8 ++++---- admin/spec/features/refund_reasons_spec.rb | 8 ++++---- admin/spec/features/return_reasons_spec.rb | 8 ++++---- admin/spec/features/roles_spec.rb | 8 ++++---- admin/spec/features/shipping_categories_spec.rb | 8 ++++---- admin/spec/features/store_credit_reasons_spec.rb | 8 ++++---- admin/spec/features/tax_categories_spec.rb | 4 ++-- 7 files changed, 26 insertions(+), 26 deletions(-) diff --git a/admin/spec/features/adjustment_reasons_spec.rb b/admin/spec/features/adjustment_reasons_spec.rb index 8af9a711ea0..14499858cfd 100644 --- a/admin/spec/features/adjustment_reasons_spec.rb +++ b/admin/spec/features/adjustment_reasons_spec.rb @@ -26,12 +26,12 @@ before do visit "/admin/adjustment_reasons#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Adjustment Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -69,12 +69,12 @@ Spree::AdjustmentReason.create(name: "Good Reason", code: 5999) visit "/admin/adjustment_reasons#{query}" find_row("Good Reason").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Adjustment Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/refund_reasons_spec.rb b/admin/spec/features/refund_reasons_spec.rb index ae5a52663eb..cb153264174 100644 --- a/admin/spec/features/refund_reasons_spec.rb +++ b/admin/spec/features/refund_reasons_spec.rb @@ -26,12 +26,12 @@ before do visit "/admin/refund_reasons/#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Refund Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -66,12 +66,12 @@ Spree::RefundReason.create(name: "Return process") visit "/admin/refund_reasons#{query}" find_row("Return process").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Refund Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/return_reasons_spec.rb b/admin/spec/features/return_reasons_spec.rb index 5932101bf41..706d9166749 100644 --- a/admin/spec/features/return_reasons_spec.rb +++ b/admin/spec/features/return_reasons_spec.rb @@ -26,12 +26,12 @@ before do visit "/admin/return_reasons#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Return Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -68,12 +68,12 @@ Spree::ReturnReason.create(name: "Good Reason") visit "/admin/return_reasons#{query}" find_row("Good Reason").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Return Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/roles_spec.rb b/admin/spec/features/roles_spec.rb index 0132e2ed205..fb09c9894e0 100644 --- a/admin/spec/features/roles_spec.rb +++ b/admin/spec/features/roles_spec.rb @@ -54,12 +54,12 @@ before do visit "/admin/roles#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Role") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -121,14 +121,14 @@ Spree::Role.create(name: "Reviewer", permission_sets: [settings_edit_permission]) visit "/admin/roles#{query}" find_row("Reviewer").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Role") expect(page).to be_axe_clean expect(Spree::Role.find_by(name: "Reviewer").permission_set_ids) .to contain_exactly(settings_edit_permission.id) end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/shipping_categories_spec.rb b/admin/spec/features/shipping_categories_spec.rb index 674ce76a00d..703de26bd66 100644 --- a/admin/spec/features/shipping_categories_spec.rb +++ b/admin/spec/features/shipping_categories_spec.rb @@ -26,12 +26,12 @@ before do visit "/admin/shipping_categories#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Shipping Category") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -66,12 +66,12 @@ Spree::ShippingCategory.create(name: "Letter Mail") visit "/admin/shipping_categories#{query}" find_row("Letter Mail").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Shipping Category") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/store_credit_reasons_spec.rb b/admin/spec/features/store_credit_reasons_spec.rb index b4b2c46aff4..50b2c11d023 100644 --- a/admin/spec/features/store_credit_reasons_spec.rb +++ b/admin/spec/features/store_credit_reasons_spec.rb @@ -26,12 +26,12 @@ before do visit "/admin/store_credit_reasons#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Store Credit Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) @@ -66,12 +66,12 @@ Spree::StoreCreditReason.create(name: "New Customer Reward") visit "/admin/store_credit_reasons#{query}" find_row("New Customer Reward").click + expect(page).to have_selector("dialog") expect(page).to have_content("Edit Store Credit Reason") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) diff --git a/admin/spec/features/tax_categories_spec.rb b/admin/spec/features/tax_categories_spec.rb index b91a24256dc..417ee5e1262 100644 --- a/admin/spec/features/tax_categories_spec.rb +++ b/admin/spec/features/tax_categories_spec.rb @@ -28,12 +28,12 @@ before do visit "/admin/tax_categories#{query}" click_on "Add new" + expect(page).to have_selector("dialog") expect(page).to have_content("New Tax Category") expect(page).to be_axe_clean end - it "opens a modal" do - expect(page).to have_selector("dialog") + it "closing the modal keeps query params" do within("dialog") { click_on "Cancel" } expect(page).not_to have_selector("dialog") expect(page.current_url).to include(query) From 7112488c7e83a3137edba403e9ebc873f230ffec Mon Sep 17 00:00:00 2001 From: Thomas von Deyen Date: Tue, 3 Dec 2024 17:10:22 +0100 Subject: [PATCH 2/2] test: Do not wait 30 seconds for a test to fail If the modal cannot be openend after 5 seconds (Capybara default wait tiem is 3 seconds) then something is wrong. We must not waste precious resources and time for a buld to fail. --- admin/spec/features/orders/show_spec.rb | 10 +++++----- .../spec/features/solidus_admin/promotions_spec.rb | 8 ++++---- .../system/solidus_promotions/admin/promotions_spec.rb | 8 ++++---- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/admin/spec/features/orders/show_spec.rb b/admin/spec/features/orders/show_spec.rb index 7b8aebd5479..986acbac250 100644 --- a/admin/spec/features/orders/show_spec.rb +++ b/admin/spec/features/orders/show_spec.rb @@ -48,7 +48,7 @@ expect(page).to have_content("Order R123456789") open_customer_menu click_on "Edit billing address" - expect(page).to have_css("dialog", wait: 30) + expect(page).to have_css("dialog", wait: 5) within("dialog") do fill_in "Name", with: "John Doe" @@ -74,7 +74,7 @@ open_customer_menu click_on "Edit shipping address" - expect(page).to have_css("dialog", wait: 30) + expect(page).to have_css("dialog", wait: 5) within("dialog") do fill_in "Name", with: "Jane Doe" @@ -119,18 +119,18 @@ expect(Spree::Order.last.line_items.count).to eq(0) find("[aria-selected]", text: "Just another product").click - expect(page).to have_content("Variant added to cart successfully", wait: 30) + expect(page).to have_content("Variant added to cart successfully", wait: 5) expect(Spree::Order.last.line_items.count).to eq(1) expect(Spree::Order.last.line_items.last.quantity).to eq(1) fill_in "line_item[quantity]", with: 4 - expect(page).to have_content("Quantity updated successfully", wait: 30) + expect(page).to have_content("Quantity updated successfully", wait: 5) expect(Spree::Order.last.line_items.last.quantity).to eq(4) accept_confirm("Are you sure?") { click_on "Delete" } - expect(page).to have_content("Line item removed successfully", wait: 30) + expect(page).to have_content("Line item removed successfully", wait: 5) expect(Spree::Order.last.line_items.count).to eq(0) expect(page).to be_axe_clean diff --git a/legacy_promotions/spec/features/solidus_admin/promotions_spec.rb b/legacy_promotions/spec/features/solidus_admin/promotions_spec.rb index 16f1439ace7..922846e2a51 100644 --- a/legacy_promotions/spec/features/solidus_admin/promotions_spec.rb +++ b/legacy_promotions/spec/features/solidus_admin/promotions_spec.rb @@ -14,13 +14,13 @@ visit "/admin/promotions" expect(page).to have_content("My active Promotion") click_on "Draft" - expect(page).to have_content("My draft Promotion", wait: 30) + expect(page).to have_content("My draft Promotion", wait: 5) click_on "Future" - expect(page).to have_content("My future Promotion", wait: 30) + expect(page).to have_content("My future Promotion", wait: 5) click_on "Expired" - expect(page).to have_content("My expired Promotion", wait: 30) + expect(page).to have_content("My expired Promotion", wait: 5) click_on "All" - expect(page).to have_content("My active Promotion", wait: 30) + expect(page).to have_content("My active Promotion", wait: 5) expect(page).to have_content("My draft Promotion") expect(page).to have_content("My future Promotion") expect(page).to have_content("My expired Promotion") diff --git a/promotions/spec/system/solidus_promotions/admin/promotions_spec.rb b/promotions/spec/system/solidus_promotions/admin/promotions_spec.rb index ddc65c9b46f..c02a8066084 100644 --- a/promotions/spec/system/solidus_promotions/admin/promotions_spec.rb +++ b/promotions/spec/system/solidus_promotions/admin/promotions_spec.rb @@ -14,13 +14,13 @@ visit "/admin/solidus/promotions" expect(page).to have_content("My active Promotion") click_on "Draft" - expect(page).to have_content("My draft Promotion", wait: 30) + expect(page).to have_content("My draft Promotion", wait: 5) click_on "Future" - expect(page).to have_content("My future Promotion", wait: 30) + expect(page).to have_content("My future Promotion", wait: 5) click_on "Expired" - expect(page).to have_content("My expired Promotion", wait: 30) + expect(page).to have_content("My expired Promotion", wait: 5) click_on "All" - expect(page).to have_content("My active Promotion", wait: 30) + expect(page).to have_content("My active Promotion", wait: 5) expect(page).to have_content("My draft Promotion") expect(page).to have_content("My future Promotion") expect(page).to have_content("My expired Promotion")