From 111addc892cf1e3b9e7333f6cc80afba0e496c9f Mon Sep 17 00:00:00 2001
From: Joe Woodward <jwoodward@gitlab.com>
Date: Tue, 22 Oct 2024 16:28:11 +0000
Subject: [PATCH] Update file locking QA feature spec
We recently introduced a new merge request check that looks for locked
files.
The check ensures that none of the paths are locked by anyone other than
the author.
It looks like we forgot to update the spec. The spec didn't fail because
the author was also locking the file so we weren't tripping this check.
We also introduced a new merge commit diff mode which interacts with
this check. The diff mode will ignore changes from merge commits that
are present in all parents i.e.
If the merge request author merged the target_branch into the
source_branch after making changes in the source_branch:
Previously, we would include all of those files as changed_paths even if
the merge request author hadn't modifed them through fixing merge
conflicts or manually amending the commit.
Now, we only include the merged changes in the changed_paths if the user
fixed a merge conflict or manually amended the file.
So what was previously happening was the check during the MR phase would
return zero locked paths but then when you merge and gitlab pushes the
changes to the repository we would run the same checks on the merge
commit and that would be flagged because it's no longer the merge
request author but the person who merged the MR.
When we enable the new merge commit diff mode this change isn't
evaluated after being merged so we do not receive any error and the test
fails.
I've updated the test to now check that the path lock check has failed.
---
qa/qa/page/merge_request/show.rb | 4 ++++
.../3_create/repository/file_locking_spec.rb | 15 ++++++---------
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb
index dee0ba2e824be..d6462df2488f2 100644
--- a/qa/qa/page/merge_request/show.rb
+++ b/qa/qa/page/merge_request/show.rb
@@ -322,6 +322,10 @@ def set_to_auto_merge!
click_element('merge-button', text: 'Set to auto-merge')
end
+ def auto_mergeable?
+ has_element?('merge-button', text: 'Set to auto-merge', wait: 10)
+ end
+
def merged?
# Reloads the page at this point to avoid the problem of the merge status failing to update
# That's the transient UX issue this test is checking for, so if the MR is merged but the UI still shows the
diff --git a/qa/qa/specs/features/ee/browser_ui/3_create/repository/file_locking_spec.rb b/qa/qa/specs/features/ee/browser_ui/3_create/repository/file_locking_spec.rb
index f3913545420dd..42fb628f55215 100644
--- a/qa/qa/specs/features/ee/browser_ui/3_create/repository/file_locking_spec.rb
+++ b/qa/qa/specs/features/ee/browser_ui/3_create/repository/file_locking_spec.rb
@@ -66,7 +66,7 @@ module QA
end
end
- it 'creates a merge request and fails to merge', :blocking,
+ it 'merge request is blocked when a path is locked by another user', :blocking,
testcase: 'https://gitlab.com/gitlab-org/gitlab/-/quality/test_cases/347770' do
push as_user: user_one, max_attempts: 3, branch: 'test'
@@ -76,15 +76,16 @@ module QA
source_branch: 'test',
target_branch: project.default_branch)
+ Flow::Login.sign_in(as: user_one, skip_page_validation: true)
go_to_file
click_lock
- Flow::Login.sign_in(as: user_one, skip_page_validation: true)
+
merge_request.visit!
Page::MergeRequest::Show.perform do |merge_request|
- merge_request.try_to_merge!
- expect(page).to have_text("locked by @#{admin_username}", wait: 30)
- expect(merge_request).to have_merge_button
+ expect(merge_request).to have_content('Merge blocked: 1 check failed', wait: 20)
+ expect(merge_request).to have_content('All paths must be unlocked')
+ expect(merge_request).to be_auto_mergeable
end
end
@@ -150,10 +151,6 @@ def expect_no_error_on_push(as_user:, for_file: 'file')
push as_user: as_user, max_attempts: 3, branch: project.default_branch, file: for_file
end.not_to raise_error
end
-
- def admin_username
- create(:user, username: Runtime::User.username).username
- end
end
end
end
--
GitLab