From cbdda549893141d5b708802746b0904f9208ba85 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 11 Jan 2019 17:14:46 +1100 Subject: [PATCH 1/4] DRY specs with new helper --- .../admin/bulk_order_management_spec.rb | 19 ++++--------- .../admin/bulk_product_update_spec.rb | 28 +++++-------------- spec/features/admin/customers_spec.rb | 4 +-- spec/features/admin/order_cycles_spec.rb | 21 ++++---------- spec/features/admin/product_import_spec.rb | 5 ++-- spec/features/admin/subscriptions_spec.rb | 4 +-- spec/features/admin/variant_overrides_spec.rb | 18 ++++-------- spec/support/request/admin_helper.rb | 12 ++++++++ 8 files changed, 40 insertions(+), 71 deletions(-) diff --git a/spec/features/admin/bulk_order_management_spec.rb b/spec/features/admin/bulk_order_management_spec.rb index 7ec845cdac..d6410daef7 100644 --- a/spec/features/admin/bulk_order_management_spec.rb +++ b/spec/features/admin/bulk_order_management_spec.rb @@ -4,6 +4,7 @@ feature %q{ As an Administrator I want to be able to manage orders in bulk } , js: true do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -189,11 +190,7 @@ feature %q{ context "modifying the weight/volume of a line item" do it "price is altered" do visit '/admin/orders/bulk_management' - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Weight/Volume").click - find("div#columns-dropdown div.menu div.menu_item", text: "Price").click - # hide dropdown - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Weight/Volume", "Price" within "tr#li_#{li1.id}" do expect(page).to have_field "price", with: "50.00" fill_in "final_weight_volume", :with => 2000 @@ -210,9 +207,7 @@ feature %q{ context "modifying the quantity of a line item" do it "price is altered" do visit '/admin/orders/bulk_management' - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Price").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Price" within "tr#li_#{li1.id}" do expect(page).to have_field "price", with: "#{format("%.2f",li1.price * 5)}" fill_in "quantity", :with => 6 @@ -224,9 +219,7 @@ feature %q{ context "modifying the quantity of a line item" do it "weight/volume is altered" do visit '/admin/orders/bulk_management' - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Weight/Volume").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Weight/Volume" within "tr#li_#{li1.id}" do expect(page).to have_field "final_weight_volume", with: "#{li1.final_weight_volume.round}" fill_in "quantity", :with => 6 @@ -246,9 +239,7 @@ feature %q{ expect(page).to have_selector "th", :text => "QUANTITY" expect(page).to have_selector "th", :text => "MAX" - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Producer").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Producer" expect(page).to have_no_selector "th", :text => "PRODUCER" expect(page).to have_selector "th", :text => "NAME" diff --git a/spec/features/admin/bulk_product_update_spec.rb b/spec/features/admin/bulk_product_update_spec.rb index 5ea80b6e5e..96259bfe55 100644 --- a/spec/features/admin/bulk_product_update_spec.rb +++ b/spec/features/admin/bulk_product_update_spec.rb @@ -4,6 +4,7 @@ feature %q{ As an Administrator I want to be able to manage products in bulk } , js: true do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -46,9 +47,7 @@ feature %q{ p2 = FactoryBot.create(:product, available_on: Date.current-1) visit spree.admin_products_path - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Available On").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Available On" expect(page).to have_field "available_on", with: p1.available_on.strftime("%F %T") expect(page).to have_field "available_on", with: p2.available_on.strftime("%F %T") @@ -246,12 +245,7 @@ feature %q{ quick_login_as_admin visit spree.admin_products_path - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Available On").click - find("div#columns-dropdown div.menu div.menu_item", text: /^Category?/).click - find("div#columns-dropdown div.menu div.menu_item", text: "Inherits Properties?").click - find("div#columns-dropdown div.menu div.menu_item", text: "SKU").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Available On", /^Category?/, "Inherits Properties?", "SKU" within "tr#p_#{p.id}" do expect(page).to have_field "product_name", with: p.name @@ -318,9 +312,7 @@ feature %q{ expect(page).to have_selector "a.view-variants", count: 1 find("a.view-variants").trigger('click') - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "SKU").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "SKU" expect(page).to have_field "variant_sku", with: "VARIANTSKU" expect(page).to have_field "variant_price", with: "3.0" @@ -566,9 +558,7 @@ feature %q{ visit spree.admin_products_path - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Available On").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Available On" expect(page).to have_selector "th", :text => "NAME" expect(page).to have_selector "th", :text => "PRODUCER" @@ -576,9 +566,7 @@ feature %q{ expect(page).to have_selector "th", :text => "ON HAND" expect(page).to have_selector "th", :text => "AV. ON" - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: /^.{0,1}Producer$/).click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns /^.{0,1}Producer$/ expect(page).to have_no_selector "th", :text => "PRODUCER" expect(page).to have_selector "th", :text => "NAME" @@ -701,9 +689,7 @@ feature %q{ v = p.variants.first visit spree.admin_products_path - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Available On").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Available On" within "tr#p_#{p.id}" do expect(page).to have_field "product_name", with: p.name diff --git a/spec/features/admin/customers_spec.rb b/spec/features/admin/customers_spec.rb index 0bf9dbda26..65a27d6ef8 100644 --- a/spec/features/admin/customers_spec.rb +++ b/spec/features/admin/customers_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' feature 'Customers' do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -66,8 +67,7 @@ feature 'Customers' do # Toggling columns expect(page).to have_selector "th.email" expect(page).to have_content customer1.email - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "Email").click + toggle_columns "Email" expect(page).to have_no_selector "th.email" expect(page).to have_no_content customer1.email diff --git a/spec/features/admin/order_cycles_spec.rb b/spec/features/admin/order_cycles_spec.rb index acaab40a7a..fb1c8731ad 100644 --- a/spec/features/admin/order_cycles_spec.rb +++ b/spec/features/admin/order_cycles_spec.rb @@ -4,6 +4,7 @@ feature %q{ As an administrator I want to manage order cycles }, js: true do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -45,10 +46,7 @@ feature %q{ page.should have_selector "#listing_order_cycles tr.order-cycle-#{oc5.id}.closed" page.should have_selector "#listing_order_cycles tr.order-cycle-#{oc6.id}.closed" - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Producers").click - find("div#columns-dropdown div.menu div.menu_item", text: "Shops").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Producers", "Shops" # And I should see all the details for an order cycle within('table#listing_order_cycles tbody tr:nth-child(2)') do @@ -245,10 +243,7 @@ feature %q{ oc = OrderCycle.last - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Producers").click - find("div#columns-dropdown div.menu div.menu_item", text: "Shops").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Producers", "Shops" expect(page).to have_input "oc#{oc.id}[name]", value: "Plums & Avos" expect(page).to have_input "oc#{oc.id}[orders_open_at]", value: order_cycle_opening_time @@ -388,10 +383,7 @@ feature %q{ oc = OrderCycle.last - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Producers").click - find("div#columns-dropdown div.menu div.menu_item", text: "Shops").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Producers", "Shops" expect(page).to have_input "oc#{oc.id}[name]", value: "Plums & Avos" expect(page).to have_input "oc#{oc.id}[orders_open_at]", value: order_cycle_opening_time @@ -705,10 +697,7 @@ feature %q{ expect(page).to have_selector "tr.order-cycle-#{oc_user_coordinating.id}" expect(page).to_not have_selector "tr.order-cycle-#{oc_for_other_user.id}" - find("div#columns-dropdown", :text => "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Producers").click - find("div#columns-dropdown div.menu div.menu_item", text: "Shops").click - find("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Producers", "Shops" # The order cycle should show all enterprises in the order cycle page.should have_selector 'td.producers', text: supplier_managed.name diff --git a/spec/features/admin/product_import_spec.rb b/spec/features/admin/product_import_spec.rb index 3a337d93dd..c1b87b1f33 100644 --- a/spec/features/admin/product_import_spec.rb +++ b/spec/features/admin/product_import_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' require 'open_food_network/permissions' feature "Product Import", js: true do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -156,9 +157,7 @@ feature "Product Import", js: true do expect(page).to have_field "product_name", with: carrots.name expect(page).to have_field "product_name", with: potatoes.name - find("div#columns-dropdown", text: "COLUMNS").click - find("div#columns-dropdown div.menu div.menu_item", text: "Import").click - find("div#columns-dropdown", text: "COLUMNS").click + toggle_columns "Import" within "tr#p_#{carrots.id} td.import_date" do expect(page).to have_content Time.zone.now.year diff --git a/spec/features/admin/subscriptions_spec.rb b/spec/features/admin/subscriptions_spec.rb index edbd884cf8..37b496919d 100644 --- a/spec/features/admin/subscriptions_spec.rb +++ b/spec/features/admin/subscriptions_spec.rb @@ -1,6 +1,7 @@ require 'spec_helper' feature 'Subscriptions' do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -62,8 +63,7 @@ feature 'Subscriptions' do # Toggling columns expect(page).to have_selector "th.customer" expect(page).to have_content subscription.customer.email - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "Customer").click + toggle_columns "Customer" expect(page).to have_no_selector "th.customer" expect(page).to have_no_content subscription.customer.email diff --git a/spec/features/admin/variant_overrides_spec.rb b/spec/features/admin/variant_overrides_spec.rb index f2b0bb916b..a10617e393 100644 --- a/spec/features/admin/variant_overrides_spec.rb +++ b/spec/features/admin/variant_overrides_spec.rb @@ -6,6 +6,7 @@ feature %q{ I want to override the stock level and price of those products Without affecting other hubs that share the same products }, js: true do + include AdminHelper include AuthenticationWorkflow include WebHelper @@ -112,9 +113,7 @@ feature %q{ expect(page).to have_selector "tr#v_#{variant_related.id}" # Show/Hide products - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "Hide").click - first("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Hide" expect(page).to have_selector "tr#v_#{variant.id}" expect(page).to have_selector "tr#v_#{variant_related.id}" within "tr#v_#{variant.id}" do click_button 'Hide' end @@ -134,9 +133,7 @@ feature %q{ end it "creates new overrides" do - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "SKU").click - first("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "SKU" fill_in "variant-overrides-#{variant.id}-sku", with: 'NEWSKU' fill_in "variant-overrides-#{variant.id}-price", with: '777.77' @@ -287,15 +284,10 @@ feature %q{ # Any new fields added to the VO model need to be added to this test it "deletes overrides when values are cleared" do - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "Enable Stock Reset?").click - first("div#columns-dropdown div.menu div.menu_item", text: "Tags").click - first("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Enable Stock Reset?", "Tags" # Clearing values by 'inheriting' - first("div#columns-dropdown", :text => "COLUMNS").click - first("div#columns-dropdown div.menu div.menu_item", text: "Inherit?").click - first("div#columns-dropdown", :text => "COLUMNS").click + toggle_columns "Inherit?" check "variant-overrides-#{variant3.id}-inherit" # Clearing values manually diff --git a/spec/support/request/admin_helper.rb b/spec/support/request/admin_helper.rb index ff0c50b6f5..e5163594f4 100644 --- a/spec/support/request/admin_helper.rb +++ b/spec/support/request/admin_helper.rb @@ -2,4 +2,16 @@ module AdminHelper def have_admin_menu_item(menu_item_name) have_selector "ul[data-hook='admin_tabs'] li", text: menu_item_name end + + def toggle_columns(*labels) + # open dropdown + find("div#columns-dropdown", text: "COLUMNS").click + + labels.each do |label| + find("div#columns-dropdown div.menu div.menu_item", text: label).click + end + + # close dropdown + find("div#columns-dropdown", text: "COLUMNS").click + end end From 223982f57194c3d4ce613341e1d6b4de16f5dfed Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 11 Jan 2019 17:35:57 +1100 Subject: [PATCH 2/4] Prepare spec for Chrome testing --- spec/features/admin/variant_overrides_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/features/admin/variant_overrides_spec.rb b/spec/features/admin/variant_overrides_spec.rb index a10617e393..b4633cc7d7 100644 --- a/spec/features/admin/variant_overrides_spec.rb +++ b/spec/features/admin/variant_overrides_spec.rb @@ -289,6 +289,11 @@ feature %q{ # Clearing values by 'inheriting' toggle_columns "Inherit?" check "variant-overrides-#{variant3.id}-inherit" + # Hide the Inherit column again. When that column is visible, the + # size of the Tags column is too short and tags can't be removed. + # This is a bug and the next line can be removed once it is fixed: + # https://github.com/openfoodfoundation/openfoodnetwork/issues/3310 + toggle_columns "Inherit?" # Clearing values manually fill_in "variant-overrides-#{variant.id}-price", with: '' From e89a836e728bd545d89e835ebb9c40e981343e89 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 11 Jan 2019 17:39:50 +1100 Subject: [PATCH 3/4] Remove forgotten code from spec Commit e4ebeb8a29b167822568cbfabe0ae785881051d3 forgot to remove this code. --- spec/features/admin/enterprise_user_spec.rb | 1 - spec/support/request/admin_helper.rb | 4 ---- 2 files changed, 5 deletions(-) diff --git a/spec/features/admin/enterprise_user_spec.rb b/spec/features/admin/enterprise_user_spec.rb index 8ef52ee6b4..1408fddc02 100644 --- a/spec/features/admin/enterprise_user_spec.rb +++ b/spec/features/admin/enterprise_user_spec.rb @@ -6,7 +6,6 @@ feature %q{ } do include AuthenticationWorkflow include WebHelper - include AdminHelper let!(:user) { create_enterprise_user } let!(:supplier1) { create(:supplier_enterprise, name: 'Supplier 1') } diff --git a/spec/support/request/admin_helper.rb b/spec/support/request/admin_helper.rb index e5163594f4..d83e4b85ef 100644 --- a/spec/support/request/admin_helper.rb +++ b/spec/support/request/admin_helper.rb @@ -1,8 +1,4 @@ module AdminHelper - def have_admin_menu_item(menu_item_name) - have_selector "ul[data-hook='admin_tabs'] li", text: menu_item_name - end - def toggle_columns(*labels) # open dropdown find("div#columns-dropdown", text: "COLUMNS").click From 4abacb3691413bc686c506002a7aafdda063a1f9 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 11 Jan 2019 17:42:41 +1100 Subject: [PATCH 4/4] Remove unused spec helpers They all seem to have been used in the past. People forgot to remove them after removing the using spec code. --- spec/support/request/web_helper.rb | 58 ------------------------------ 1 file changed, 58 deletions(-) diff --git a/spec/support/request/web_helper.rb b/spec/support/request/web_helper.rb index 71e43599f4..fd9cceb3b0 100644 --- a/spec/support/request/web_helper.rb +++ b/spec/support/request/web_helper.rb @@ -26,11 +26,6 @@ module WebHelper have_selector selector end - def current_path_should_be path - current_path = URI.parse(current_url).path - expect(page).to have_current_path path - end - def fill_in_fields(field_values) field_values.each do |key, value| begin @@ -46,73 +41,20 @@ module WebHelper page.find_by_id(from).find("option[value='#{value}']").select_option end - def should_have_failed - page.status_code.should == 200 - errors.count.should > 0 - end - - def successful? - page.status_code.should == 200 - errors.count.should == 0 - flash_message.should include 'successful' - end - - def updated_field(field, value) - wait_for_ajax - yield(field, value) - rescue Capybara::TimeoutError - flunk "Expected #{field} to update to #{value}." - end - - def updated_css(css) - wait_for_ajax - yield(css) - rescue Capybara::TimeoutError - flunk "Expected updated css: #{css}." - end - - def user_nav - find('div.user/div.name').text - end - def flash_message find('.flash', visible: false).text.strip end - def errors - all('.error') - end - - def property_name - find('.property-name').text - end - - def error_messages - errors.map(&:text) - end - def handle_js_confirm(accept=true) page.execute_script "window.confirm = function(msg) { return #{!!accept }; }" yield end - def click_dialog_button(button_content) - page.find(:xpath, "//div[@class=\"ui-dialog-buttonset\"]//span[contains(text(),\"#{button_content}\")]").click - end - def visit_delete(url) response = Capybara.current_session.driver.delete url click_link 'redirected' if response.status == 302 end - def trigger_manual_event(field_selector, event = 'change') - page.execute_script("$('#{field_selector}').trigger('#{event}');") - end - - def dirty_form_dialog - DirtyFormDialog.new(page) - end - def set_i18n_locale(locale = 'en') page.execute_script("I18n.locale = '#{locale}'") end