From e941e96f33019fe86d84114ffb573872e12ee14a Mon Sep 17 00:00:00 2001 From: YarNayar Date: Mon, 28 Nov 2016 15:50:31 +0300 Subject: [PATCH] Make notification_service spec DRYer by making some tests reusable The spec for 'participatns' notifications in notification_service for both issue and merge_request is pretty simple but it was fully copied without sagnificant changes 8 times (!!!) for EVERY situation you need to notify this group of user which caused to file to extra growth and not being very DRY. And I love DRY. Since the spec is already too messy and most likely gonna to increase the size nearest time, I decided to refactor those parts. --- ...tification_service_spec-to-make-it-DRY.yml | 4 + spec/services/notification_service_spec.rb | 315 +++++------------- 2 files changed, 91 insertions(+), 228 deletions(-) create mode 100644 changelogs/unreleased/get-rid-of-water-from-notification_service_spec-to-make-it-DRY.yml diff --git a/changelogs/unreleased/get-rid-of-water-from-notification_service_spec-to-make-it-DRY.yml b/changelogs/unreleased/get-rid-of-water-from-notification_service_spec-to-make-it-DRY.yml new file mode 100644 index 00000000000..f60417d185e --- /dev/null +++ b/changelogs/unreleased/get-rid-of-water-from-notification_service_spec-to-make-it-DRY.yml @@ -0,0 +1,4 @@ +--- +title: Make notification_service spec DRYer by making test reusable +merge_request: +author: YarNayar diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index f3e80ac22a0..260e1f3fb68 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -33,6 +33,49 @@ describe NotificationService, services: true do end end + # Next shared examples are intended to test notifications of "participants" + # + # they take the following parameters: + # * issuable + # * notification trigger + # * participant + # + shared_examples 'participating by note notification' do + it 'emails the participant' do + create(:note_on_issue, noteable: issuable, project_id: project.id, note: 'anything', author: participant) + + notification_trigger + + should_email(participant) + end + end + + shared_examples 'participating by assignee notification' do + it 'emails the participant' do + issuable.update_attribute(:assignee, participant) + + notification_trigger + + should_email(participant) + end + end + + shared_examples 'participating by author notification' do + it 'emails the participant' do + issuable.author = participant + + notification_trigger + + should_email(participant) + end + end + + shared_examples_for 'participating notifications' do + it_should_behave_like 'participating by note notification' + it_should_behave_like 'participating by author notification' + it_should_behave_like 'participating by assignee notification' + end + describe 'Keys' do describe '#new_key' do let!(:key) { create(:personal_key) } @@ -539,32 +582,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - issue.update_attribute(:assignee, @u_lazy_participant) - notification.reassigned_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } - - before { notification.reassigned_issue(issue, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - issue.author = @u_lazy_participant - notification.reassigned_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { issue } + let(:notification_trigger) { notification.reassigned_issue(issue, @u_disabled) } end end @@ -671,32 +692,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - issue.update_attribute(:assignee, @u_lazy_participant) - notification.close_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } - - before { notification.close_issue(issue, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - issue.author = @u_lazy_participant - notification.close_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { issue } + let(:notification_trigger) { notification.close_issue(issue, @u_disabled) } end end @@ -723,32 +722,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - issue.update_attribute(:assignee, @u_lazy_participant) - notification.reopen_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: issue, project_id: issue.project_id, note: 'anything', author: @u_lazy_participant) } - - before { notification.reopen_issue(issue, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - issue.author = @u_lazy_participant - notification.reopen_issue(issue, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { issue } + let(:notification_trigger) { notification.reopen_issue(issue, @u_disabled) } end end end @@ -809,31 +786,28 @@ describe NotificationService, services: true do end context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.new_merge_request(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } + it_should_behave_like 'participating by assignee notification' do + let(:participant) { create(:user, username: 'user-participant')} + let(:issuable) { merge_request } + let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } end - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.new_merge_request(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } + it_should_behave_like 'participating by note notification' do + let(:participant) { create(:user, username: 'user-participant')} + let(:issuable) { merge_request } + let(:notification_trigger) { notification.new_merge_request(merge_request, @u_disabled) } end context 'by author' do + let(:participant) { create(:user, username: 'user-participant')} + before do - merge_request.author = @u_lazy_participant + merge_request.author = participant merge_request.save notification.new_merge_request(merge_request, @u_disabled) end - it { should_not_email(@u_lazy_participant) } + it { should_not_email(participant) } end end end @@ -868,33 +842,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.reassigned_merge_request(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.reassigned_merge_request(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - merge_request.author = @u_lazy_participant - merge_request.save - notification.reassigned_merge_request(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { merge_request } + let(:notification_trigger) { notification.reassigned_merge_request(merge_request, @u_disabled) } end end @@ -965,33 +916,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.close_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.close_mr(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - merge_request.author = @u_lazy_participant - merge_request.save - notification.close_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { merge_request } + let(:notification_trigger) { notification.close_mr(merge_request, @u_disabled) } end end @@ -1032,33 +960,10 @@ describe NotificationService, services: true do should_not_email(@u_watcher) end - context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.merge_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.merge_mr(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - merge_request.author = @u_lazy_participant - merge_request.save - notification.merge_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { merge_request } + let(:notification_trigger) { notification.merge_mr(merge_request, @u_disabled) } end end @@ -1085,33 +990,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.reopen_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.reopen_mr(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - merge_request.author = @u_lazy_participant - merge_request.save - notification.reopen_mr(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { merge_request } + let(:notification_trigger) { notification.reopen_mr(merge_request, @u_disabled) } end end @@ -1131,33 +1013,10 @@ describe NotificationService, services: true do should_not_email(@u_lazy_participant) end - context 'participating' do - context 'by assignee' do - before do - merge_request.update_attribute(:assignee, @u_lazy_participant) - notification.resolve_all_discussions(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end - - context 'by note' do - let!(:note) { create(:note_on_issue, noteable: merge_request, project_id: project.id, note: 'anything', author: @u_lazy_participant) } - - before { notification.resolve_all_discussions(merge_request, @u_disabled) } - - it { should_email(@u_lazy_participant) } - end - - context 'by author' do - before do - merge_request.author = @u_lazy_participant - merge_request.save - notification.resolve_all_discussions(merge_request, @u_disabled) - end - - it { should_email(@u_lazy_participant) } - end + it_behaves_like 'participating notifications' do + let(:participant) { create(:user, username: 'user-participant') } + let(:issuable) { merge_request } + let(:notification_trigger) { notification.resolve_all_discussions(merge_request, @u_disabled) } end end end