From 939e9bdde144849cbc11091985bca0a27f6e75ac Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 1 Aug 2017 12:55:57 -0400 Subject: [PATCH] Make the 'issuables list meta-data' shared example less dangerous This shared example would take an object's database ID and create a number of objects based on it. If for some reason the ID were a high number, like 20, this would create `20 + 21 + 22` objects. Not only was this dangerous from a performance perspective, it was entirely unnecessary, as the behavior it was testing is already well-tested in the unit test for the underlying object. For a controller test, which is what's including this shared example, all we need to do is verify that the assigned object contains the correct `id => object` Hash, which is what we now test for. --- .../projects/merge_requests_controller_spec.rb | 4 +++- .../issuables_list_metadata_shared_examples.rb | 11 +++-------- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 0bfe83f1d98..216c59d3ae9 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -106,7 +106,7 @@ describe Projects::MergeRequestsController do end describe 'GET index' do - let!(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } def get_merge_requests(page = nil) get :index, @@ -150,6 +150,8 @@ describe Projects::MergeRequestsController do context 'when filtering by opened state' do context 'with opened merge requests' do it 'lists those merge requests' do + expect(merge_request).to be_persisted + get_merge_requests expect(assigns(:merge_requests)).to include(merge_request) diff --git a/spec/support/issuables_list_metadata_shared_examples.rb b/spec/support/issuables_list_metadata_shared_examples.rb index 3406e4c3161..1004c895bb4 100644 --- a/spec/support/issuables_list_metadata_shared_examples.rb +++ b/spec/support/issuables_list_metadata_shared_examples.rb @@ -11,10 +11,6 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| end @issuable_ids << issuable.id - - issuable.id.times { create(:note, noteable: issuable, project: issuable.project) } - (issuable.id + 1).times { create(:award_emoji, :downvote, awardable: issuable) } - (issuable.id + 2).times { create(:award_emoji, :upvote, awardable: issuable) } end end @@ -27,10 +23,9 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil| meta_data = assigns(:issuable_meta_data) - @issuable_ids.each do |id| - expect(meta_data[id].notes_count).to eq(id) - expect(meta_data[id].downvotes).to eq(id + 1) - expect(meta_data[id].upvotes).to eq(id + 2) + aggregate_failures do + expect(meta_data.keys).to match_array(@issuable_ids) + expect(meta_data.values).to all(be_kind_of(Issuable::IssuableMeta)) end end