Commit 10dc561a authored by Rémy Coutable's avatar Rémy Coutable

Fix a flaky merge_requests_controller_spec.rb

The problem was as follows:

1. Private build access level wasn't carried over to the forked project
   (in ProjectForksHelper).
1. That made the spec fail on first attempt since the project member
   would indeed get pipelines in the JSON response.
1. On second attempt, because `@merge_request` is memoized in
   `Projects::MergeRequests::ApplicationController`, the MergeCommitDiff
   returned by `Ci::PipelinesForMergeRequestFinder#all_pipelines_for_merge_request`
   would be empty since it would look for the first attempt's MR diffs.
1. This made the test pass because there would be no pipelines found,
   but it wasn't actually testing that private builds would prevent the
   pipelines from being returned, it was only passing due to a
   combination of rspec-retry and memoization of a controller's instance variable.

Due to this reason, I think we should disable retrying controller tests.
Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent d15efc8e
......@@ -28,11 +28,15 @@ module ProjectForksHelper
unless params[:target_project] || params[:using_service]
target_level = [project.visibility_level, namespace.visibility_level].min
visibility_level = Gitlab::VisibilityLevel.closest_allowed_level(target_level)
# Builds and MRs can't have higher visibility level than repository access level.
builds_access_level = [project.builds_access_level, project.repository_access_level].min
params[:target_project] =
create(:project,
(:repository if create_repository),
visibility_level: visibility_level, creator: user, namespace: namespace)
visibility_level: visibility_level,
builds_access_level: builds_access_level,
creator: user, namespace: namespace)
end
service = Projects::ForkService.new(project, user, params)
......
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