Merge branch 'rails-save-bang-10' into 'master'

Fix Rails/SaveBang cop for specs in spec/features/merge_requests/*

See merge request gitlab-org/gitlab!38591

.rubocop_todo.yml

@@ -1325,14 +1325,6 @@ Rails/SaveBang:
     - 'spec/features/issues/user_filters_issues_spec.rb'
     - 'spec/features/issues/user_sees_live_update_spec.rb'
     - 'spec/features/issues/user_sorts_issues_spec.rb'
-    - 'spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb'
-    - 'spec/features/merge_request/user_posts_diff_notes_spec.rb'
-    - 'spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb'
-    - 'spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb'
-    - 'spec/features/merge_request/user_sees_discussions_spec.rb'
-    - 'spec/features/merge_request/user_sees_merge_widget_spec.rb'
-    - 'spec/features/merge_request/user_sees_versions_spec.rb'
-    - 'spec/features/merge_requests/user_mass_updates_spec.rb'
     - 'spec/features/profiles/emails_spec.rb'
     - 'spec/features/profiles/password_spec.rb'
     - 'spec/features/profiles/personal_access_tokens_spec.rb'

changelogs/unreleased/rails-save-bang-10.yml

+---
+title: Refactor spec/features/merge_requests/* to fix Rails/SaveBang Cop
+merge_request: 38591
+author: Rajendra Kadam
+type: fixed

spec/features/merge_request/user_merges_when_pipeline_succeeds_spec.rb

@@ -154,7 +154,7 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do
 
     context 'view merge request with MWPS enabled but automatically merge fails' do
       before do
-        merge_request.update(
+        merge_request.update!(
           merge_user: merge_request.author,
           merge_error: 'Something went wrong.'
         )
@@ -173,7 +173,7 @@ RSpec.describe 'Merge request > User merges when pipeline succeeds', :js do
 
     context 'view merge request with MWPS enabled but automatically merge fails' do
       before do
-        merge_request.update(
+        merge_request.update!(
           merge_user: merge_request.author,
           merge_error: 'Something went wrong.'
         )

spec/features/merge_request/user_posts_diff_notes_spec.rb

@@ -193,7 +193,7 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
 
   context 'when the MR only supports legacy diff notes' do
     before do
-      merge_request.merge_request_diff.update(start_commit_sha: nil)
+      merge_request.merge_request_diff.update!(start_commit_sha: nil)
       visit diffs_project_merge_request_path(project, merge_request, view: 'inline')
     end
 

spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do
     before do
       project.add_maintainer(user)
       sign_in(user)
-      note.destroy
+      note.destroy!
       visit_merge_request
     end
 

spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb

@@ -26,7 +26,7 @@ RSpec.describe 'Merge request > User cherry-picks', :js do
     context 'without a merge commit' do
       before do
         merge_request.merge_commit_sha = nil
-        merge_request.save
+        merge_request.save!
       end
 
       it 'does not show a Cherry-pick button' do

spec/features/merge_request/user_sees_discussions_spec.rb

@@ -13,8 +13,8 @@ RSpec.describe 'Merge request > User sees threads', :js do
   end
 
   describe "Diff discussions" do
-    let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create(diff_refs: outdated_diff_refs) }
-    let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create }
+    let!(:old_merge_request_diff) { merge_request.merge_request_diffs.create!(diff_refs: outdated_diff_refs) }
+    let!(:new_merge_request_diff) { merge_request.merge_request_diffs.create! }
     let!(:outdated_discussion) { create(:diff_note_on_merge_request, project: project, noteable: merge_request, position: outdated_position).to_discussion }
     let!(:active_discussion) { create(:diff_note_on_merge_request, noteable: merge_request, project: project).to_discussion }
     let(:outdated_position) do

spec/features/merge_request/user_sees_merge_widget_spec.rb

@@ -302,7 +302,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
 
   context 'view merge request with MWPS enabled but automatically merge fails' do
     before do
-      merge_request.update(
+      merge_request.update!(
         auto_merge_enabled: true,
         auto_merge_strategy: AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS,
         merge_user: merge_request.author,
@@ -324,7 +324,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
 
   context 'view merge request with MWPS enabled but automatically merge fails' do
     before do
-      merge_request.update(
+      merge_request.update!(
         merge_when_pipeline_succeeds: true,
         merge_user: merge_request.author,
         merge_error: 'Something went wrong'
@@ -345,9 +345,9 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
 
   context 'view merge request where fast-forward merge is not possible' do
     before do
-      project.update(merge_requests_ff_only_enabled: true)
+      project.update!(merge_requests_ff_only_enabled: true)
 
-      merge_request.update(
+      merge_request.update!(
         merge_user: merge_request.author,
         merge_status: :cannot_be_merged
       )
@@ -380,19 +380,19 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
     end
   end
 
-  context 'user can merge into source project but cannot push to fork', :js do
-    let(:fork_project) { create(:project, :public, :repository) }
+  context 'user can merge into target project but cannot push to fork', :js do
+    let(:forked_project) { fork_project(project, nil, repository: true) }
     let(:user2) { create(:user) }
 
     before do
       project.add_maintainer(user2)
       sign_out(:user)
       sign_in(user2)
-      merge_request.update(target_project: fork_project)
+      merge_request.update!(source_project: forked_project)
       visit project_merge_request_path(project, merge_request)
     end
 
-    it 'user can merge into the source project' do
+    it 'user can merge into the target project', :sidekiq_inline do
       expect(page).to have_button('Merge', disabled: false)
     end
 
@@ -409,7 +409,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
       project.add_developer(user2)
       sign_out(:user)
       sign_in(user2)
-      merge_request.update(
+      merge_request.update!(
         source_project: forked_project,
         target_project: project,
         merge_params: { 'force_remove_source_branch' => '1' }
@@ -879,7 +879,7 @@ RSpec.describe 'Merge request > User sees merge widget', :js do
     let!(:pipeline) { create(:ci_pipeline, status: 'success', sha: sha, project: project, ref: merge_request.source_branch) }
 
     before do
-      project.update(
+      project.update!(
         visibility_level: Gitlab::VisibilityLevel::PUBLIC,
         public_builds: false
       )

spec/features/merge_request/user_sees_versions_spec.rb

@@ -5,14 +5,14 @@ require 'spec_helper'
 RSpec.describe 'Merge request > User sees versions', :js do
   let(:merge_request) do
     create(:merge_request).tap do |mr|
-      mr.merge_request_diff.destroy
+      mr.merge_request_diff.destroy!
     end
   end
   let(:project) { merge_request.source_project }
   let(:user) { project.creator }
-  let!(:merge_request_diff1) { merge_request.merge_request_diffs.create(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
-  let!(:merge_request_diff2) { merge_request.merge_request_diffs.create(head_commit_sha: nil) }
-  let!(:merge_request_diff3) { merge_request.merge_request_diffs.create(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
+  let!(:merge_request_diff1) { merge_request.merge_request_diffs.create!(head_commit_sha: '6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9') }
+  let!(:merge_request_diff2) { merge_request.merge_request_diffs.create!(head_commit_sha: nil) }
+  let!(:merge_request_diff3) { merge_request.merge_request_diffs.create!(head_commit_sha: '5937ac0a7beb003549fc5fd26fc247adbce4a52e') }
   let!(:params) { {} }
 
   before do

spec/features/merge_requests/user_mass_updates_spec.rb

@@ -95,7 +95,7 @@ RSpec.describe 'Merge requests > User mass updates', :js do
     describe 'unset milestone' do
       before do
         merge_request.milestone = milestone
-        merge_request.save
+        merge_request.save!
         visit project_merge_requests_path(project)
       end