From c291601d9f5faaf484dcf4af227e7857a2b1fa5f Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:04:40 +0000 Subject: [PATCH 1/8] Fix rubocop warnings in enterprise_fee_summary report spec --- .../reports/enterprise_fee_summaries_spec.rb | 33 ++++++++++++++----- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb index be03f7d393..edbd48bcd0 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb @@ -86,8 +86,10 @@ xfeature "enterprise fee summaries", js: true do end context "when logged in as enterprise user" do - let!(:order) { create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor) } - + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end let(:current_user) { distributor.owner } it "shows available options for the enterprise" do @@ -102,8 +104,10 @@ xfeature "enterprise fee summaries", js: true do end context "when logged in as admin" do - let!(:order) { create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor) } - + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end let(:current_user) { create(:admin_user) } it "generates file with data for all enterprises" do @@ -115,9 +119,14 @@ xfeature "enterprise fee summaries", js: true do end context "when logged in as enterprise user" do - let!(:order) { create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor) } - let!(:other_order) { create(:completed_order_with_fees, order_cycle: other_order_cycle, distributor: other_distributor) } - + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end + let!(:other_order) do + create(:completed_order_with_fees, order_cycle: other_order_cycle, + distributor: other_distributor) + end let(:current_user) { distributor.owner } it "generates file with data for the enterprise" do @@ -134,8 +143,14 @@ xfeature "enterprise fee summaries", js: true do let!(:second_distributor) { create(:distributor_enterprise) } let!(:second_order_cycle) { create(:simple_order_cycle, coordinator: second_distributor) } - let!(:order) { create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor) } - let!(:second_order) { create(:completed_order_with_fees, order_cycle: second_order_cycle, distributor: second_distributor) } + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end + let!(:second_order) do + create(:completed_order_with_fees, order_cycle: second_order_cycle, + distributor: second_distributor) + end let(:current_user) { create(:admin_user) } From 4795832db598c2cd512a59f78a0d9c538bf52137 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:06:15 +0000 Subject: [PATCH 2/8] Add spec DownloadsHelper to support testing csv downloads with headless chrome --- spec/support/downloads_helper.rb | 38 ++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 spec/support/downloads_helper.rb diff --git a/spec/support/downloads_helper.rb b/spec/support/downloads_helper.rb new file mode 100644 index 0000000000..a0980e8900 --- /dev/null +++ b/spec/support/downloads_helper.rb @@ -0,0 +1,38 @@ +module DownloadsHelper + TIMEOUT = 10 + PATH = Rails.root.join("tmp", "downloads") + + def downloaded_filename + wait_for_download + downloaded_filenames.first + end + + def downloaded_content + wait_for_download + File.read(downloaded_filename) + end + + def clear_downloads + FileUtils.rm_f(downloaded_filenames) + end + + private + + def downloaded_filenames + Dir[PATH.join("*")] + end + + def wait_for_download + Timeout.timeout(TIMEOUT) do + sleep 0.1 until downloaded? + end + end + + def downloaded? + !downloading? && downloaded_filenames.any? + end + + def downloading? + downloaded_filenames.grep(/\.crdownload$/).any? + end +end From 4f803eba7eb38b2c823c2d14452e6a1a718e3e77 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:08:24 +0000 Subject: [PATCH 3/8] Add around blocks to clear downloads directory before and after every test --- .../reports/enterprise_fee_summaries_spec.rb | 116 ++++++++++-------- 1 file changed, 62 insertions(+), 54 deletions(-) diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb index edbd48bcd0..354ca9d185 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb @@ -98,74 +98,82 @@ xfeature "enterprise fee summaries", js: true do end end - describe "smoke test for generation of report based on permissions" do - before do - visit spree.new_admin_reports_enterprise_fee_summary_path + describe "csv downloads" do + around do |example| + clear_downloads + example.run + clear_downloads end - context "when logged in as admin" do + describe "smoke test for generation of report based on permissions" do + before do + visit spree.new_admin_reports_enterprise_fee_summary_path + end + + context "when logged in as admin" do + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end + let(:current_user) { create(:admin_user) } + + it "generates file with data for all enterprises" do + check I18n.t("filters.report_format_csv", scope: i18n_scope) + click_on I18n.t("filters.generate_report", scope: i18n_scope) + expect(page.response_headers['Content-Type']).to eq "text/csv" + expect(page.body).to have_content(distributor.name) + end + end + + context "when logged in as enterprise user" do + let!(:order) do + create(:completed_order_with_fees, order_cycle: order_cycle, + distributor: distributor) + end + let!(:other_order) do + create(:completed_order_with_fees, order_cycle: other_order_cycle, + distributor: other_distributor) + end + let(:current_user) { distributor.owner } + + it "generates file with data for the enterprise" do + check I18n.t("filters.report_format_csv", scope: i18n_scope) + click_on I18n.t("filters.generate_report", scope: i18n_scope) + expect(page.response_headers['Content-Type']).to eq "text/csv" + expect(page.body).to have_content(distributor.name) + expect(page.body).not_to have_content(other_distributor.name) + end + end + end + + describe "smoke test for filtering report based on filters" do + let!(:second_distributor) { create(:distributor_enterprise) } + let!(:second_order_cycle) { create(:simple_order_cycle, coordinator: second_distributor) } + let!(:order) do create(:completed_order_with_fees, order_cycle: order_cycle, distributor: distributor) end + let!(:second_order) do + create(:completed_order_with_fees, order_cycle: second_order_cycle, + distributor: second_distributor) + end + let(:current_user) { create(:admin_user) } - it "generates file with data for all enterprises" do + before do + visit spree.new_admin_reports_enterprise_fee_summary_path + end + + it "generates file with data for selected order cycle" do + select order_cycle.name, from: "report_order_cycle_ids" check I18n.t("filters.report_format_csv", scope: i18n_scope) click_on I18n.t("filters.generate_report", scope: i18n_scope) expect(page.response_headers['Content-Type']).to eq "text/csv" expect(page.body).to have_content(distributor.name) + expect(page.body).not_to have_content(second_distributor.name) end end - - context "when logged in as enterprise user" do - let!(:order) do - create(:completed_order_with_fees, order_cycle: order_cycle, - distributor: distributor) - end - let!(:other_order) do - create(:completed_order_with_fees, order_cycle: other_order_cycle, - distributor: other_distributor) - end - let(:current_user) { distributor.owner } - - it "generates file with data for the enterprise" do - check I18n.t("filters.report_format_csv", scope: i18n_scope) - click_on I18n.t("filters.generate_report", scope: i18n_scope) - expect(page.response_headers['Content-Type']).to eq "text/csv" - expect(page.body).to have_content(distributor.name) - expect(page.body).not_to have_content(other_distributor.name) - end - end - end - - describe "smoke test for filtering report based on filters" do - let!(:second_distributor) { create(:distributor_enterprise) } - let!(:second_order_cycle) { create(:simple_order_cycle, coordinator: second_distributor) } - - let!(:order) do - create(:completed_order_with_fees, order_cycle: order_cycle, - distributor: distributor) - end - let!(:second_order) do - create(:completed_order_with_fees, order_cycle: second_order_cycle, - distributor: second_distributor) - end - - let(:current_user) { create(:admin_user) } - - before do - visit spree.new_admin_reports_enterprise_fee_summary_path - end - - it "generates file with data for selected order cycle" do - select order_cycle.name, from: "report_order_cycle_ids" - check I18n.t("filters.report_format_csv", scope: i18n_scope) - click_on I18n.t("filters.generate_report", scope: i18n_scope) - expect(page.response_headers['Content-Type']).to eq "text/csv" - expect(page.body).to have_content(distributor.name) - expect(page.body).not_to have_content(second_distributor.name) - end end def i18n_scope From c36f7e003b82e70881b8394ff997a92951d24c36 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:09:55 +0000 Subject: [PATCH 4/8] Adapt assertions to new csv file download logic --- .../reports/enterprise_fee_summaries_spec.rb | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb index 354ca9d185..4af4dab6c2 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb @@ -120,8 +120,9 @@ xfeature "enterprise fee summaries", js: true do it "generates file with data for all enterprises" do check I18n.t("filters.report_format_csv", scope: i18n_scope) click_on I18n.t("filters.generate_report", scope: i18n_scope) - expect(page.response_headers['Content-Type']).to eq "text/csv" - expect(page.body).to have_content(distributor.name) + + expect(downloaded_filename).to include ".csv" + expect(downloaded_content).to have_content(distributor.name) end end @@ -139,9 +140,11 @@ xfeature "enterprise fee summaries", js: true do it "generates file with data for the enterprise" do check I18n.t("filters.report_format_csv", scope: i18n_scope) click_on I18n.t("filters.generate_report", scope: i18n_scope) - expect(page.response_headers['Content-Type']).to eq "text/csv" - expect(page.body).to have_content(distributor.name) - expect(page.body).not_to have_content(other_distributor.name) + + expect(downloaded_filename).to include ".csv" + csv_content = downloaded_content + expect(csv_content).to have_content(distributor.name) + expect(csv_content).not_to have_content(other_distributor.name) end end end @@ -169,9 +172,11 @@ xfeature "enterprise fee summaries", js: true do select order_cycle.name, from: "report_order_cycle_ids" check I18n.t("filters.report_format_csv", scope: i18n_scope) click_on I18n.t("filters.generate_report", scope: i18n_scope) - expect(page.response_headers['Content-Type']).to eq "text/csv" - expect(page.body).to have_content(distributor.name) - expect(page.body).not_to have_content(second_distributor.name) + + expect(downloaded_filename).to include ".csv" + csv_content = downloaded_content + expect(csv_content).to have_content(distributor.name) + expect(csv_content).not_to have_content(second_distributor.name) end end end From d9304916ccf0127f714f25c906b02e9d71064674 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:43:28 +0000 Subject: [PATCH 5/8] Add chrome driver config to make testing csv downloads with headless chrome possible --- spec/spec_helper.rb | 13 ++++++++++++- spec/support/downloads_helper.rb | 7 +++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 68a523c4af..103fbd96d6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -46,7 +46,17 @@ Capybara.register_driver :chrome do |app| options = Selenium::WebDriver::Chrome::Options.new( args: %w[headless disable-gpu no-sandbox window-size=1280,768] ) - Capybara::Selenium::Driver.new(app, browser: :chrome, options: options) + driver = Capybara::Selenium::Driver.new(app, browser: :chrome, options: options) + + ### Allow file downloads in headless chrome + ### https://bugs.chromium.org/p/chromium/issues/detail?id=696481#c89 + bridge = driver.browser.send(:bridge) + path = '/session/:session_id/chromium/send_command' + path[':session_id'] = bridge.session_id + bridge.http.call(:post, path, cmd: 'Page.setDownloadBehavior', + params: { behavior: 'allow', downloadPath: DownloadsHelper.path.to_s }) + + driver end Capybara.default_max_wait_time = 30 @@ -145,6 +155,7 @@ RSpec.configure do |config| config.include ActionView::Helpers::DateHelper config.include OpenFoodNetwork::DelayedJobHelper config.include OpenFoodNetwork::PerformanceHelper + config.include DownloadsHelper # FactoryBot require 'factory_bot_rails' diff --git a/spec/support/downloads_helper.rb b/spec/support/downloads_helper.rb index a0980e8900..732bf22769 100644 --- a/spec/support/downloads_helper.rb +++ b/spec/support/downloads_helper.rb @@ -1,6 +1,9 @@ module DownloadsHelper TIMEOUT = 10 - PATH = Rails.root.join("tmp", "downloads") + + def self.path + Rails.root.join("tmp", "downloads") + end def downloaded_filename wait_for_download @@ -19,7 +22,7 @@ module DownloadsHelper private def downloaded_filenames - Dir[PATH.join("*")] + Dir[DownloadsHelper.path.join("*")] end def wait_for_download From e10264af864cfe4b2b88bd6f0890f6788f149610 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Mon, 25 Mar 2019 15:43:53 +0000 Subject: [PATCH 6/8] Remove xfeature from enterprise fees summmary report spec --- spec/features/admin/reports/enterprise_fee_summaries_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb index 4af4dab6c2..1b235ecf06 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb @@ -1,6 +1,6 @@ require "spec_helper" -xfeature "enterprise fee summaries", js: true do +feature "enterprise fee summaries", js: true do include AuthenticationWorkflow include WebHelper From 7cc5940e19e3db57dd695750b4b1c72468967430 Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 28 Mar 2019 23:01:21 +0000 Subject: [PATCH 7/8] Load DownloadsHelper only for feature specs --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 103fbd96d6..3b0ae37fb7 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -155,7 +155,7 @@ RSpec.configure do |config| config.include ActionView::Helpers::DateHelper config.include OpenFoodNetwork::DelayedJobHelper config.include OpenFoodNetwork::PerformanceHelper - config.include DownloadsHelper + config.include DownloadsHelper, type: :feature # FactoryBot require 'factory_bot_rails' From 40124b74215728c0b8ed028717c2944e509ed56d Mon Sep 17 00:00:00 2001 From: luisramos0 Date: Thu, 28 Mar 2019 23:01:58 +0000 Subject: [PATCH 8/8] Improve downloads_helper clear_downloads method --- .../admin/reports/enterprise_fee_summaries_spec.rb | 4 +--- spec/support/downloads_helper.rb | 10 ++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb index 1b235ecf06..844eeadf03 100644 --- a/spec/features/admin/reports/enterprise_fee_summaries_spec.rb +++ b/spec/features/admin/reports/enterprise_fee_summaries_spec.rb @@ -100,9 +100,7 @@ feature "enterprise fee summaries", js: true do describe "csv downloads" do around do |example| - clear_downloads - example.run - clear_downloads + with_empty_downloads_folder { example.run } end describe "smoke test for generation of report based on permissions" do diff --git a/spec/support/downloads_helper.rb b/spec/support/downloads_helper.rb index 732bf22769..2adb6fcbe5 100644 --- a/spec/support/downloads_helper.rb +++ b/spec/support/downloads_helper.rb @@ -15,8 +15,10 @@ module DownloadsHelper File.read(downloaded_filename) end - def clear_downloads - FileUtils.rm_f(downloaded_filenames) + def with_empty_downloads_folder + remove_downloaded_files + yield + remove_downloaded_files end private @@ -38,4 +40,8 @@ module DownloadsHelper def downloading? downloaded_filenames.grep(/\.crdownload$/).any? end + + def remove_downloaded_files + FileUtils.rm_f(downloaded_filenames) + end end