diff options
author | Robert Speicher <rspeicher@gmail.com> | 2017-08-01 12:55:57 -0400 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2017-08-01 12:55:57 -0400 |
commit | 939e9bdde144849cbc11091985bca0a27f6e75ac (patch) | |
tree | d6b1075fddd5ec546a1bd199a67b5391f6a64342 | |
parent | 4c77c30fbfb3734fd893f7cfe540fa595ea6539c (diff) | |
download | gitlab-ce-939e9bdde144849cbc11091985bca0a27f6e75ac.tar.gz |
Make the 'issuables list meta-data' shared example less dangerousrs-issuables-list-metadata-shared-examples
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.
-rw-r--r-- | spec/controllers/projects/merge_requests_controller_spec.rb | 4 | ||||
-rw-r--r-- | spec/support/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 |