Commit 45ce5ff7 authored by Rémy Coutable's avatar Rémy Coutable

Fix Projects::MergeRequests::DiffsController specs

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: default avatarRémy Coutable <remy@rymai.me>
parent 5dede999
# frozen_string_literal: true # frozen_string_literal: true
class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController class Projects::MergeRequests::DiffsController < Projects::MergeRequests::ApplicationController
include DiffForPath
include DiffHelper include DiffHelper
include RendersNotes include RendersNotes
......
...@@ -4,10 +4,11 @@ describe Projects::MergeRequests::DiffsController do ...@@ -4,10 +4,11 @@ describe Projects::MergeRequests::DiffsController do
include ProjectForksHelper include ProjectForksHelper
let(:project) { create(:project, :repository) } 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) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
before do before do
project.add_maintainer(user)
sign_in(user) sign_in(user)
end end
...@@ -114,16 +115,6 @@ describe Projects::MergeRequests::DiffsController do ...@@ -114,16 +115,6 @@ describe Projects::MergeRequests::DiffsController do
expect(paths).to include(existing_path) expect(paths).to include(existing_path)
end end
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 end
context 'when the user cannot view the merge request' do context 'when the user cannot view the merge request' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment