From accc6535cdb53ced2578b955350877050d0ffa75 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Wed, 10 Mar 2021 15:46:29 +0100 Subject: [PATCH 1/3] Remove excluded factories for FactoryBot/InlineAssociation They have been fixed but it was forgotten to remove them. --- .rubocop_manual_todo.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 4572078e721fcb..f34bdb333d48e0 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -50,9 +50,6 @@ FactoryBot/InlineAssociation: - 'spec/factories/events.rb' - 'spec/factories/git_wiki_commit_details.rb' - 'spec/factories/gitaly/commit.rb' - - 'spec/factories/go_module_commits.rb' - - 'spec/factories/go_module_versions.rb' - - 'spec/factories/go_modules.rb' - 'spec/factories/group_group_links.rb' - 'spec/factories/import_export_uploads.rb' -- GitLab From e93aabe1b4dac1c4fd7ce9a545afb28e608a2b54 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Mon, 15 Mar 2021 12:58:22 +0100 Subject: [PATCH 2/3] Refactor specs for Vulnerabilities::Feedback to use `build_stubbed` This commit not only speeds up specs (instead of `create`) but makes it easier to unset issue/merge_request association. --- .../vulnerabilities/feedback_entity_spec.rb | 36 ++++++++----------- 1 file changed, 15 insertions(+), 21 deletions(-) diff --git a/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb b/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb index f444d412d22dfa..aff780fcae5dd2 100644 --- a/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb +++ b/ee/spec/serializers/vulnerabilities/feedback_entity_spec.rb @@ -15,13 +15,11 @@ end describe '#as_json' do - let(:feedback) { build(:vulnerability_feedback, :issue, project: project) } + let(:feedback) { build_stubbed(:vulnerability_feedback, :issue, project: project) } it { is_expected.to include(:created_at, :project_id, :author, :category, :feedback_type) } - context 'feedback type is issue' do - let(:feedback) { build(:vulnerability_feedback, :issue, project: project) } - + context 'when feedback type is issue' do context 'when issue is present' do it 'exposes the issue iid' do is_expected.to include(:issue_iid) @@ -45,15 +43,11 @@ end context 'when there is no current user' do - let(:entity) { described_class.represent(feedback, request: request) } - before do allow(request).to receive(:current_user).and_return(nil) allow(feedback).to receive(:author).and_return(nil) end - subject { entity.as_json } - it 'does not include fields related to current user' do is_expected.not_to include(:issue_url) is_expected.not_to include(:destroy_vulnerability_feedback_dismissal_path) @@ -62,9 +56,9 @@ end context 'when issue is not present' do - let(:feedback) { build(:vulnerability_feedback, feedback_type: :issue, project: project, issue: nil) } - it 'does not expose issue information' do + feedback.issue = nil + is_expected.not_to include(:issue_iid) is_expected.not_to include(:issue_url) end @@ -81,8 +75,8 @@ end end - context 'feedback type is merge_request' do - let(:feedback) { build(:vulnerability_feedback, :merge_request, project: project) } + context 'when feedback type is merge_request' do + let(:feedback) { build_stubbed(:vulnerability_feedback, :merge_request, project: project) } context 'when merge request is present' do it 'exposes the merge request iid' do @@ -107,9 +101,9 @@ end context 'when merge request is not present' do - let(:feedback) { build(:vulnerability_feedback, :merge_request, project: project, merge_request: nil) } - it 'does not expose merge request information' do + feedback.merge_request = nil + is_expected.not_to include(:merge_request_iid) is_expected.not_to include(:merge_request_path) end @@ -126,8 +120,8 @@ end end - context 'feedback type is dismissal' do - let(:feedback) { create(:vulnerability_feedback, :dismissal, project: project) } + context 'when feedback type is dismissal' do + let(:feedback) { build_stubbed(:vulnerability_feedback, :dismissal, project: project) } context 'when not allowed to destroy vulnerability feedback' do before do @@ -152,13 +146,13 @@ end context 'when comment is not present' do - let(:feedback) { build(:vulnerability_feedback, :dismissal, project: project) } + let(:feedback) { build_stubbed(:vulnerability_feedback, :dismissal, project: project) } it { is_expected.not_to include(:comment_details) } end context 'when comment is present' do - let(:feedback) { build(:vulnerability_feedback, :comment, project: project) } + let(:feedback) { build_stubbed(:vulnerability_feedback, :comment, project: project) } it 'exposes comment information' do expect(subject).to include(:comment_details) @@ -169,7 +163,7 @@ end context 'when finding_uuid is not present' do - let(:feedback) { build(:vulnerability_feedback, :issue, project: project) } + let(:feedback) { build_stubbed(:vulnerability_feedback, :issue, project: project) } it 'has a nil finding_uuid' do expect(subject[:finding_uuid]).to be_nil @@ -177,8 +171,8 @@ end context 'when finding_uuid is present' do - let_it_be(:finding) { create(:vulnerabilities_finding) } - let(:feedback) { create(:vulnerability_feedback, finding_uuid: finding.uuid, project: project) } + let(:finding) { build_stubbed(:vulnerabilities_finding) } + let(:feedback) { build_stubbed(:vulnerability_feedback, finding_uuid: finding.uuid, project: project) } it 'exposes finding_uuid' do expect(subject[:finding_uuid]).to eq(finding.uuid) -- GitLab From 7d54eebc7c05694e37b330e4047d83af5086c2a7 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Mon, 15 Mar 2021 13:04:56 +0100 Subject: [PATCH 3/3] Fix remaining EE offenses for FactoryBot/InlineAssociation --- .rubocop_manual_todo.yml | 2 -- ee/spec/factories/merge_request_blocks.rb | 4 ++-- ee/spec/factories/vulnerabilities/feedback.rb | 6 +++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index f34bdb333d48e0..9b20af55520e31 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -44,8 +44,6 @@ Graphql/Descriptions: # WIP See https://gitlab.com/gitlab-org/gitlab/-/issues/267606 FactoryBot/InlineAssociation: Exclude: - - 'ee/spec/factories/merge_request_blocks.rb' - - 'ee/spec/factories/vulnerabilities/feedback.rb' - 'spec/factories/atlassian_identities.rb' - 'spec/factories/events.rb' - 'spec/factories/git_wiki_commit_details.rb' diff --git a/ee/spec/factories/merge_request_blocks.rb b/ee/spec/factories/merge_request_blocks.rb index 73102b48c73264..ace88cd1b51286 100644 --- a/ee/spec/factories/merge_request_blocks.rb +++ b/ee/spec/factories/merge_request_blocks.rb @@ -2,7 +2,7 @@ FactoryBot.define do factory :merge_request_block do - blocking_merge_request { create(:merge_request) } - blocked_merge_request { create(:merge_request) } + blocking_merge_request { association(:merge_request) } + blocked_merge_request { association(:merge_request) } end end diff --git a/ee/spec/factories/vulnerabilities/feedback.rb b/ee/spec/factories/vulnerabilities/feedback.rb index 4fdf5cc97d19c4..338d9af2083670 100644 --- a/ee/spec/factories/vulnerabilities/feedback.rb +++ b/ee/spec/factories/vulnerabilities/feedback.rb @@ -12,7 +12,7 @@ author issue { nil } merge_request { nil } - pipeline { create(:ci_pipeline, project: project) } + pipeline { association(:ci_pipeline, project: project) } feedback_type { 'dismissal' } category { 'sast' } project_fingerprint { generate(:project_fingerprint) } @@ -30,12 +30,12 @@ trait :issue do feedback_type { 'issue' } - issue { create(:issue, project: project) } + issue { association(:issue, project: project) } end trait :merge_request do feedback_type { 'merge_request' } - merge_request { create(:merge_request, source_project: project) } + merge_request { association(:merge_request, source_project: project) } end trait :sast do -- GitLab