From 6594529cda7522754dd7959a905f12757636c687 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 3 Dec 2018 16:00:11 +0100 Subject: [PATCH 1/5] Fix manager invitation controller spec --- .../manager_invitations_controller_spec.rb | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/spec/controllers/admin/manager_invitations_controller_spec.rb b/spec/controllers/admin/manager_invitations_controller_spec.rb index 0b86ef3b66..071566e3a4 100644 --- a/spec/controllers/admin/manager_invitations_controller_spec.rb +++ b/spec/controllers/admin/manager_invitations_controller_spec.rb @@ -26,20 +26,26 @@ module Admin end context "signing up a new user" do + let(:manager_invitation) { instance_double(ManagerInvitationJob) } + before do setup_email controller.stub spree_current_user: admin end - it "creates a new user, sends an invitation email, and returns the user id" do - expect do - spree_post :create, {email: 'un.registered@email.com', enterprise_id: enterprise.id} - end.to send_confirmation_instructions + it 'enqueues an invitation email' do + allow(ManagerInvitationJob) + .to receive(:new).with(enterprise.id, kind_of(Integer)) { manager_invitation } + + expect(Delayed::Job).to receive(:enqueue).with(manager_invitation) + + spree_post :create, { email: 'un.registered@email.com', enterprise_id: enterprise.id } + end + + it "returns the user id" do + spree_post :create, { email: 'un.registered@email.com', enterprise_id: enterprise.id } new_user = Spree::User.find_by_email('un.registered@email.com') - - expect(new_user.reset_password_token).to_not be_nil - expect(response.status).to eq 200 expect(json_response['user']).to eq new_user.id end end From 5845f80af0ffacda44d2b4ec8470da4a52dfbb99 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 3 Dec 2018 16:03:38 +0100 Subject: [PATCH 2/5] Remove commented out code Because git keeps it for you. --- app/views/admin/account/show.html.haml | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/app/views/admin/account/show.html.haml b/app/views/admin/account/show.html.haml index 07c540ba1f..e46fb9623f 100644 --- a/app/views/admin/account/show.html.haml +++ b/app/views/admin/account/show.html.haml @@ -45,26 +45,3 @@ %td.text-center   %td= t(:total).upcase %td.text-right= invoice_total_for(invoice) - - --# - if @enterprises.empty? --# %h4 No enterprises to display --# --# - @enterprises.each do |enterprise| --# %h2= enterprise.name --# %table --# %thead --# %th Begins --# %th Ends --# %th Sells --# %th Trial? --# %th Turnover --# %th Bill --# - enterprise.billable_periods.each do |billable_period| --# %tr --# %td= billable_period.begins_at.in_time_zone.strftime("%F %T") --# %td= billable_period.ends_at.in_time_zone.strftime("%F %T") --# %td= billable_period.sells --# %td= billable_period.trial? --# %td= billable_period.display_turnover --# %td= billable_period.display_bill From 064d9cf1836ba0cad459fc747967de82a3c761e2 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 3 Dec 2018 16:52:06 +0100 Subject: [PATCH 3/5] Do not hide flaky tests under endless retries If it fails, needs fixing, not hiding the problem and making the test suite longer. --- spec/features/consumer/authentication_spec.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index be29540e94..95a3e9d4a0 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -1,14 +1,9 @@ require 'spec_helper' -feature "Authentication", js: true, retry: 3 do +feature "Authentication", js: true do include UIComponentHelper include OpenFoodNetwork::EmailHelper - # Attempt to address intermittent failures in these specs - around do |example| - Capybara.using_wait_time(120) { example.run } - end - describe "login" do let(:user) { create(:user, password: "password", password_confirmation: "password") } From 25eb29721573c8861190c83f894e1d6c353d23d1 Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Mon, 3 Dec 2018 16:54:07 +0100 Subject: [PATCH 4/5] Perform delivery when checking deliveries in specs --- spec/features/admin/users_spec.rb | 2 ++ spec/features/consumer/account/settings_spec.rb | 2 ++ spec/features/consumer/authentication_spec.rb | 2 ++ 3 files changed, 6 insertions(+) diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index 1e57a95285..df4242a7a0 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -33,6 +33,8 @@ feature "Managing users" do let(:user) { create :user, confirmed_at: nil } it "displays success" do + ActionMailer::Base.perform_deliveries = true + visit spree.edit_admin_user_path user expect do diff --git a/spec/features/consumer/account/settings_spec.rb b/spec/features/consumer/account/settings_spec.rb index 1c3880c974..9c8d0775e7 100644 --- a/spec/features/consumer/account/settings_spec.rb +++ b/spec/features/consumer/account/settings_spec.rb @@ -21,6 +21,8 @@ feature "Account Settings", js: true do end it "allows the user to update their email address" do + ActionMailer::Base.perform_deliveries = true + fill_in 'user_email', with: 'new@email.com' expect do diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index 95a3e9d4a0..d98847accd 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -71,6 +71,8 @@ feature "Authentication", js: true do end scenario "Signing up successfully" do + ActionMailer::Base.perform_deliveries = true + setup_email fill_in "Email", with: "test@foo.com" fill_in "Choose a password", with: "test12345" From 6af8544c52943dd77ff18717abc65ea16acbb0fe Mon Sep 17 00:00:00 2001 From: Pau Perez Date: Wed, 5 Dec 2018 10:15:06 +0100 Subject: [PATCH 5/5] Restore perform_deliveries value in tests --- spec/features/admin/users_spec.rb | 6 ++-- .../consumer/account/settings_spec.rb | 28 +++++++++---------- spec/features/consumer/authentication_spec.rb | 20 ++++++------- spec/support/email_helper.rb | 12 ++++++++ 4 files changed, 40 insertions(+), 26 deletions(-) diff --git a/spec/features/admin/users_spec.rb b/spec/features/admin/users_spec.rb index df4242a7a0..862e0fe6e0 100644 --- a/spec/features/admin/users_spec.rb +++ b/spec/features/admin/users_spec.rb @@ -32,9 +32,11 @@ feature "Managing users" do describe "resending confirmation email", js: true do let(:user) { create :user, confirmed_at: nil } - it "displays success" do - ActionMailer::Base.perform_deliveries = true + around do |example| + performing_deliveries { example.run } + end + it "displays success" do visit spree.edit_admin_user_path user expect do diff --git a/spec/features/consumer/account/settings_spec.rb b/spec/features/consumer/account/settings_spec.rb index 9c8d0775e7..2edc45cab1 100644 --- a/spec/features/consumer/account/settings_spec.rb +++ b/spec/features/consumer/account/settings_spec.rb @@ -21,23 +21,23 @@ feature "Account Settings", js: true do end it "allows the user to update their email address" do - ActionMailer::Base.perform_deliveries = true + performing_deliveries do + fill_in 'user_email', with: 'new@email.com' - fill_in 'user_email', with: 'new@email.com' + expect do + click_button I18n.t(:update) + end.to send_confirmation_instructions - expect do - click_button I18n.t(:update) - end.to send_confirmation_instructions + sent_mail = ActionMailer::Base.deliveries.last + expect(sent_mail.to).to eq ['new@email.com'] - sent_mail = ActionMailer::Base.deliveries.last - expect(sent_mail.to).to eq ['new@email.com'] - - expect(find(".alert-box.success").text.strip).to eq "#{I18n.t('spree.account_updated')} ×" - user.reload - expect(user.email).to eq 'old@email.com' - expect(user.unconfirmed_email).to eq 'new@email.com' - click_link I18n.t('spree.users.show.tabs.settings') - expect(page).to have_content I18n.t('spree.users.show.unconfirmed_email', unconfirmed_email: 'new@email.com') + expect(find(".alert-box.success").text.strip).to eq "#{I18n.t('spree.account_updated')} ×" + user.reload + expect(user.email).to eq 'old@email.com' + expect(user.unconfirmed_email).to eq 'new@email.com' + click_link I18n.t('spree.users.show.tabs.settings') + expect(page).to have_content I18n.t('spree.users.show.unconfirmed_email', unconfirmed_email: 'new@email.com') + end end it "allows the user to change their password" do diff --git a/spec/features/consumer/authentication_spec.rb b/spec/features/consumer/authentication_spec.rb index d98847accd..ed0c87d66d 100644 --- a/spec/features/consumer/authentication_spec.rb +++ b/spec/features/consumer/authentication_spec.rb @@ -71,17 +71,17 @@ feature "Authentication", js: true do end scenario "Signing up successfully" do - ActionMailer::Base.perform_deliveries = true + performing_deliveries do + setup_email + fill_in "Email", with: "test@foo.com" + fill_in "Choose a password", with: "test12345" + fill_in "Confirm password", with: "test12345" - setup_email - fill_in "Email", with: "test@foo.com" - fill_in "Choose a password", with: "test12345" - fill_in "Confirm password", with: "test12345" - - expect do - click_signup_button - expect(page).to have_content I18n.t('devise.user_registrations.spree_user.signed_up_but_unconfirmed') - end.to send_confirmation_instructions + expect do + click_signup_button + expect(page).to have_content I18n.t('devise.user_registrations.spree_user.signed_up_but_unconfirmed') + end.to send_confirmation_instructions + end end end diff --git a/spec/support/email_helper.rb b/spec/support/email_helper.rb index aea0cee76d..f622785425 100644 --- a/spec/support/email_helper.rb +++ b/spec/support/email_helper.rb @@ -7,5 +7,17 @@ module OpenFoodNetwork def setup_email Spree::Config[:mails_from] = "test@ofn.example.org" end + + # Ensures the value `perform_deliveries` had is restored. This saves us + # from messing up with the test suite's global state which is cause of + # trouble. + def performing_deliveries + old_value = ActionMailer::Base.perform_deliveries + ActionMailer::Base.perform_deliveries = true + + yield + + ActionMailer::Base.perform_deliveries = old_value + end end end