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.
This commit is contained in:
parent
4c77c30fbf
commit
939e9bdde1
2 changed files with 6 additions and 9 deletions
|
@ -106,7 +106,7 @@ describe Projects::MergeRequestsController do
|
||||||
end
|
end
|
||||||
|
|
||||||
describe 'GET index' do
|
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)
|
def get_merge_requests(page = nil)
|
||||||
get :index,
|
get :index,
|
||||||
|
@ -150,6 +150,8 @@ describe Projects::MergeRequestsController do
|
||||||
context 'when filtering by opened state' do
|
context 'when filtering by opened state' do
|
||||||
context 'with opened merge requests' do
|
context 'with opened merge requests' do
|
||||||
it 'lists those merge requests' do
|
it 'lists those merge requests' do
|
||||||
|
expect(merge_request).to be_persisted
|
||||||
|
|
||||||
get_merge_requests
|
get_merge_requests
|
||||||
|
|
||||||
expect(assigns(:merge_requests)).to include(merge_request)
|
expect(assigns(:merge_requests)).to include(merge_request)
|
||||||
|
|
|
@ -11,10 +11,6 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
|
||||||
end
|
end
|
||||||
|
|
||||||
@issuable_ids << issuable.id
|
@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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -27,10 +23,9 @@ shared_examples 'issuables list meta-data' do |issuable_type, action = nil|
|
||||||
|
|
||||||
meta_data = assigns(:issuable_meta_data)
|
meta_data = assigns(:issuable_meta_data)
|
||||||
|
|
||||||
@issuable_ids.each do |id|
|
aggregate_failures do
|
||||||
expect(meta_data[id].notes_count).to eq(id)
|
expect(meta_data.keys).to match_array(@issuable_ids)
|
||||||
expect(meta_data[id].downvotes).to eq(id + 1)
|
expect(meta_data.values).to all(be_kind_of(Issuable::IssuableMeta))
|
||||||
expect(meta_data[id].upvotes).to eq(id + 2)
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue