Skip to content

Refactor user_edits_multiple_reviewers_mr_spec.rb to get rid of duplicate code

The following discussion from !46738 (merged) should be addressed:

  • @sming-gitlab started a discussion: (+1 comment)

    You will notice this is a duplicate of line 15-16; so you might be wondering, why not combine it like so:

    image

    That's exactly what I tried but it failed 😭 For some odd reason, the reviewer_approval_rules: false just wouldn't kick in. So the gon.featuresApprovalRules continued to be true instead of false.

    image

Suggested Solution

@mlapierre: That's weird. There are plenty of tests with stub_feature_flags in a nested context. I wonder what's going on? 🤔

Ahh, the page is already loaded when the feature flag is stubbed, so the FE has a stale value.

Here's a fix:

diff --git a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb
index b6049d8d794..a6765844e65 100644
--- a/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb
+++ b/ee/spec/features/merge_request/user_edits_multiple_reviewers_mr_spec.rb
@@ -32,22 +32,21 @@
         expect(page).to have_content(rule_name)
       end
     end
-  end
 
-  context 'when reviewer_approval_rules feature flag off' do
-    let(:rule_name) { 'some-custom-rule' }
-    let!(:mr_rule) { create(:approval_merge_request_rule, merge_request: merge_request, users: [user], name: rule_name, approvals_required: 1 )}
+    context 'when reviewer_approval_rules feature flag off' do
+      before do
+        stub_feature_flags(reviewer_approval_rules: false)
 
-    before do
-      stub_feature_flags(reviewer_approval_rules: false)
-    end
+        visit edit_project_merge_request_path(target_project, merge_request)
+      end
 
-    it 'is not shown in reviewer dropdown' do
-      find('.js-reviewer-search').click
-      wait_for_requests
+      it 'is not shown in reviewer dropdown' do
+        find('.js-reviewer-search').click
+        wait_for_requests
 
-      page.within '.dropdown-menu-reviewer' do
-        expect(page).not_to have_content(rule_name)
+        page.within '.dropdown-menu-reviewer' do
+          expect(page).not_to have_content(rule_name)
+        end
       end
     end
   end