From 45ce5ff7ea19254b16a721329820712d68886298 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 14 Feb 2019 16:52:42 +0100 Subject: [PATCH] Fix Projects::MergeRequests::DiffsController specs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These specs were flaky and only passing after a retry due to how rspec-retry works. 1. The test with paths that don't exist was returning 200 on the first try, then 404 on the second, not because the paths don't exist, but because the MR IID didn't change, thus the MR couldn't be found. I decided to remove the test entirely since we don't seem to return 404 for paths that don't exist. 2. The test with a user that cannot view the merge request was failing the first time because the project owner wasn't removed with `project.team.truncate`. Changing the `let(:user)` to `create(:user)` and calling `project.add_maintainer(user)` in the `before` block fix the test. Signed-off-by: Rémy Coutable --- .../projects/merge_requests/diffs_controller.rb | 1 - .../merge_requests/diffs_controller_spec.rb | 13 ++----------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index ddffbb17ace..518d41bd3fb 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController - include DiffForPath include DiffHelper include RendersNotes diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index a6017d8e5e6..e85f32d6e30 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -4,10 +4,11 @@ describe Projects::MergeRequests::DiffsController do include ProjectForksHelper let(:project) { create(:project, :repository) } - let(:user) { project.owner } + let(:user) { create(:user) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } before do + project.add_maintainer(user) sign_in(user) end @@ -114,16 +115,6 @@ describe Projects::MergeRequests::DiffsController do expect(paths).to include(existing_path) end end - - context 'when the path does not exist in the diff' do - before do - diff_for_path(old_path: 'files/ruby/nopen.rb', new_path: 'files/ruby/nopen.rb') - end - - it 'returns a 404' do - expect(response).to have_gitlab_http_status(404) - end - end end context 'when the user cannot view the merge request' do -- GitLab