From 29692af48611fcdeedfaaa3554981ca23b650913 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. (cherry picked from commit c67f75f2db590af875c09dc1824d46d460e37297) --- 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 c1b5ad275e118d56657ff74db732f5a393c59b2f 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. (cherry picked from commit 7112488c7e83a3137edba403e9ebc873f230ffec) --- 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 b49bbd94d37..85caf2af6c8 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")