Apply backend reviewer suggestions

- Annotate using :aggregate_failures over wrapping in blocks
- DRY creating objects and specs

spec/finders/milestones_finder_spec.rb

@@ -7,12 +7,12 @@ RSpec.describe MilestonesFinder do
   let_it_be(:group) { create(:group) }
   let_it_be(:project_1) { create(:project, namespace: group) }
   let_it_be(:project_2) { create(:project, namespace: group) }
+  let_it_be(:milestone_2) { create(:milestone, group: group, start_date: now + 1.day, due_date: now + 2.days) }
+  let_it_be(:milestone_4) { create(:milestone, project: project_2, state: 'active', start_date: now + 4.days, due_date: now + 5.days) }
 
   context 'without filters' do
     let_it_be(:milestone_1) { create(:milestone, group: group, title: 'one test', start_date: now - 1.day, due_date: now) }
-    let_it_be(:milestone_2) { create(:milestone, group: group, start_date: now + 1.day, due_date: now + 2.days) }
     let_it_be(:milestone_3) { create(:milestone, project: project_1, state: 'active', start_date: now + 2.days) }
-    let_it_be(:milestone_4) { create(:milestone, project: project_2, state: 'active', start_date: now + 4.days, due_date: now + 5.days) }
     let_it_be(:milestone_5) { create(:milestone, group: group, due_date: now - 2.days) }
 
     it 'returns milestones for projects' do
@@ -37,13 +37,11 @@ RSpec.describe MilestonesFinder do
         expect(result).to contain_exactly(milestone_5, milestone_1, milestone_2, milestone_3, milestone_4)
       end
 
-      it 'orders milestones by due date' do
-        aggregate_failures do
+      it 'orders milestones by due date', :aggregate_failures do
         expect(result.first).to eq(milestone_5)
         expect(result.second).to eq(milestone_1)
         expect(result.third).to eq(milestone_2)
       end
-      end
 
       context 'when expired_last param is used' do
         let(:extra_params) { { expired_last: true } }
@@ -65,9 +63,7 @@ RSpec.describe MilestonesFinder do
 
   context 'with filters' do
     let_it_be(:milestone_1) { create(:milestone, group: group, state: 'closed', title: 'one test', start_date: now - 1.day, due_date: now) }
-    let_it_be(:milestone_2) { create(:milestone, group: group, start_date: now + 1.day, due_date: now + 2.days) }
     let_it_be(:milestone_3) { create(:milestone, project: project_1, state: 'closed', start_date: now + 2.days, due_date: now + 3.days) }
-    let_it_be(:milestone_4) { create(:milestone, project: project_2, state: 'active', start_date: now + 4.days, due_date: now + 5.days) }
 
     let(:params) do
       {

spec/models/milestone_spec.rb

@@ -149,34 +149,24 @@ RSpec.describe Milestone do
   end
 
   describe '#expired? and #expired' do
-    let_it_be(:milestone) { build(:milestone, project: project) }
-
     context "expired" do
-      before do
-        allow(milestone).to receive(:due_date).and_return(Date.today.prev_year)
-      end
+      let(:milestone) { build(:milestone, project: project, due_date: Date.today.prev_year) }
 
-      it 'returns true when due_date is in the past' do
-        aggregate_failures do
+      it 'returns true when due_date is in the past', :aggregate_failures do
         expect(milestone.expired?).to be_truthy
         expect(milestone.expired).to eq true
       end
     end
-    end
 
     context "not expired" do
-      before do
-        allow(milestone).to receive(:due_date).and_return(Date.today.next_year)
-      end
+      let(:milestone) { build(:milestone, project: project, due_date: Date.today.next_year) }
 
-      it 'returns false when due_date is in the future' do
-        aggregate_failures do
+      it 'returns false when due_date is in the future', :aggregate_failures do
         expect(milestone.expired?).to be_falsey
         expect(milestone.expired).to eq false
       end
     end
   end
-  end
 
   describe '#upcoming?' do
     it 'returns true when start_date is in the future' do
@@ -190,12 +180,6 @@ RSpec.describe Milestone do
     end
   end
 
-  describe '#can_be_closed?' do
-    let_it_be(:milestone) { build(:milestone, project: project) }
-
-    it { expect(milestone.can_be_closed?).to be_truthy }
-  end
-
   describe '#can_be_closed?' do
     let_it_be(:milestone) { build(:milestone, project: project) }
 
@@ -475,13 +459,11 @@ RSpec.describe Milestone do
     let_it_be(:milestone_6) { create(:milestone, title: 'Without due date2') }
 
     context 'ordering by due_date ascending' do
-      it 'sorts by due date in ascending order (ties broken by id in desc order)' do
-        aggregate_failures do
+      it 'sorts by due date in ascending order (ties broken by id in desc order)', :aggregate_failures do
         expect(milestone_3.id).to be < (milestone_6.id)
         expect(described_class.sort_with_expired_last('due_date_asc'))
           .to eq([milestone_1, milestone_2, milestone_6, milestone_3, milestone_4, milestone_5])
       end
-      end
 
       context 'unsupported sort_by param is presented' do
         it 'sorts by due date in ascending order by default' do
@@ -492,15 +474,13 @@ RSpec.describe Milestone do
     end
 
     context 'ordering by due_date descending' do
-      it 'sorts by due date in descending order (ties broken by id in desc order)' do
-        aggregate_failures do
+      it 'sorts by due date in descending order (ties broken by id in desc order)', :aggregate_failures do
         expect(milestone_3.id).to be < (milestone_6.id)
         expect(described_class.sort_with_expired_last('due_date_desc'))
           .to eq([milestone_2, milestone_1, milestone_6, milestone_3, milestone_5, milestone_4])
       end
     end
   end
-  end
 
   describe '.sort_by_attribute' do
     let_it_be(:milestone_1) { create(:milestone, title: 'Foo') }