From ffa1f313762f3f03505c4764f9d520aa69d9ec1a Mon Sep 17 00:00:00 2001
From: Valerie Burton <vburton@gitlab.com>
Date: Wed, 8 Jun 2022 07:18:55 +0000
Subject: [PATCH] Remove expect not_to raise_error code blocks from
personal_project_permissions_spec
---
.../end_to_end/best_practices.md | 48 +++++++++++++++++++
.../personal_project_permissions_spec.rb | 22 ++++-----
2 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/doc/development/testing_guide/end_to_end/best_practices.md b/doc/development/testing_guide/end_to_end/best_practices.md
index 45f64c9290b98..85f8beeacadee 100644
--- a/doc/development/testing_guide/end_to_end/best_practices.md
+++ b/doc/development/testing_guide/end_to_end/best_practices.md
@@ -208,6 +208,54 @@ it 'searches' do
end
```
+## Avoid multiple actions in `expect do ... raise_error` blocks
+
+When you wrap multiple actions in a single `expect do ... end.not_to raise_error` or `expect do ... end.to raise_error` block,
+it can be hard to debug the actual cause of the failure, because of how the logs are printed. Important information can be truncated
+or missing altogether.
+
+For example, if you encapsulate some actions and expectations in a private method in the test, like `expect_owner_permissions_allow_delete_issue`:
+
+```ruby
+it "has Owner role with Owner permissions" do
+ Page::Dashboard::Projects.perform do |projects|
+ projects.filter_by_name(project.name)
+
+ expect(projects).to have_project_with_access_role(project.name, 'Owner')
+ end
+
+ expect_owner_permissions_allow_delete_issue
+end
+```
+
+Then, in the method itself:
+
+```ruby
+#=> Good
+def expect_owner_permissions_allow_delete_issue
+ issue.visit!
+
+ Page::Project::Issue::Show.perform(&:delete_issue)
+
+ Page::Project::Issue::Index.perform do |index|
+ expect(index).not_to have_issue(issue)
+ end
+end
+
+#=> Bad
+def expect_owner_permissions_allow_delete_issue
+ expect do
+ issue.visit!
+
+ Page::Project::Issue::Show.perform(&:delete_issue)
+
+ Page::Project::Issue::Index.perform do |index|
+ expect(index).not_to have_issue(issue)
+ end
+ end.not_to raise_error
+end
+```
+
## Prefer to split tests across multiple files
Our framework includes a couple of parallelization mechanisms that work by executing spec files in parallel.
diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/personal_project_permissions_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/personal_project_permissions_spec.rb
index 5d0befea1cea6..fb486ab15323b 100644
--- a/qa/qa/specs/features/browser_ui/1_manage/project/personal_project_permissions_spec.rb
+++ b/qa/qa/specs/features/browser_ui/1_manage/project/personal_project_permissions_spec.rb
@@ -73,25 +73,21 @@ module QA
private
def expect_owner_permissions_allow_delete_issue
- expect do
- issue.visit!
+ issue.visit!
- Page::Project::Issue::Show.perform(&:delete_issue)
+ Page::Project::Issue::Show.perform(&:delete_issue)
- Page::Project::Issue::Index.perform do |index|
- expect(index).not_to have_issue(issue)
- end
- end.not_to raise_error
+ Page::Project::Issue::Index.perform do |index|
+ expect(index).not_to have_issue(issue)
+ end
end
def expect_maintainer_permissions_do_not_allow_delete_issue
- expect do
- issue.visit!
+ issue.visit!
- Page::Project::Issue::Show.perform do |issue|
- expect(issue).not_to have_delete_issue_button
- end
- end.not_to raise_error
+ Page::Project::Issue::Show.perform do |issue|
+ expect(issue).not_to have_delete_issue_button
+ end
end
end
end
--
GitLab