From bc6c91bd8978d39e4de29a3d106927850fd25369 Mon Sep 17 00:00:00 2001 From: Alina Mihaila Date: Mon, 31 Oct 2022 12:07:48 +0200 Subject: [PATCH 1/6] Add cop to avoid create in factories --- .rubocop.yml | 6 ++ .../factory_bot/avoid_create_in_factories.yml | 19 ++++++ .../factory_bot/avoid_create_in_factories.rb | 64 ++++++++++++++++++ spec/factories/ci/builds.rb | 4 +- .../avoid_create_in_factories_spec.rb | 66 +++++++++++++++++++ 5 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 .rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml create mode 100644 rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb create mode 100644 spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb diff --git a/.rubocop.yml b/.rubocop.yml index bf5e3c0fb567c4..6b158bcc72e977 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -483,6 +483,12 @@ RSpec/FactoryBot/AvoidCreate: - 'ee/spec/presenters/**/*.rb' - 'ee/spec/serializers/**/*.rb' +RSpec/FactoryBot/AvoidCreateInFactories: + Enabled: true + Include: + - 'spec/factories/*.rb' + - 'ee/spec/factories/*.rb' + Cop/IncludeSidekiqWorker: Enabled: true Exclude: diff --git a/.rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml b/.rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml new file mode 100644 index 00000000000000..b19b4d6835b4e6 --- /dev/null +++ b/.rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml @@ -0,0 +1,19 @@ +--- +RSpec/FactoryBot/AvoidCreateInFactories: + Details: grace period + Exclude: + - 'ee/spec/factories/groups.rb' + - 'ee/spec/factories/merge_requests.rb' + - 'ee/spec/factories/namespaces.rb' + - 'ee/spec/factories/projects.rb' + - 'ee/spec/factories/users.rb' + - 'ee/spec/factories/vulnerabilities.rb' + - 'ee/spec/factories/work_items.rb' + - 'spec/factories/environments.rb' + - 'spec/factories/groups.rb' + - 'spec/factories/issues.rb' + - 'spec/factories/merge_requests.rb' + - 'spec/factories/namespaces.rb' + - 'spec/factories/projects.rb' + - 'spec/factories/releases.rb' + - 'spec/factories/users.rb' diff --git a/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb b/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb new file mode 100644 index 00000000000000..553a2fc7527cb0 --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + class AvoidCreateInFactories < RuboCop::Cop::Base + MSG = 'Prefer inline `association` over `%{type}`. ' \ + 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories' + + FORBIDDEN_METHODS = %i[create create_list].freeze + + def_node_matcher :send_node?, <<~PATTERN + send + PATTERN + + def_node_matcher :assign_node?, <<~PATTERN + lvasgn + PATTERN + + def_node_matcher :assign_node, <<~PATTERN + (lvasgn $...) + PATTERN + + def_node_matcher :forbidden_factory_usage, <<~PATTERN + ( + send + { nil? (const { nil? (cbase) } :FactoryBot) } + ${ #{FORBIDDEN_METHODS.map(&:inspect).join(' ')} } + (sym _) + ... + ) + PATTERN + + def on_block(node) + block_body = node.body + + return unless block_body + return unless node.method_name == :after + return unless node.send_node.arguments.first.value == :create + + if send_node?(block_body) + check_node(block_body) + else + block_body.children.each { |child_node| check_node(child_node) } + end + end + + private + + def check_node(node) + node = assign_node(node)[1] if assign_node?(node) + + forbidden_method_name = forbidden_factory_usage(node) + + add_offense(node, message: format(MSG, type: forbidden_method_name)) if forbidden_method_name.present? + end + end + end + end + end +end diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 8396ef480c70d2..4013a411fac291 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -344,9 +344,7 @@ end trait :trace_artifact do - after(:create) do |build, evaluator| - create(:ci_job_artifact, :trace, job: build) - end + job_artifacts_trace { association(:ci_job_artifact, :trace) } end trait :unarchived_trace_artifact do diff --git a/spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb b/spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb new file mode 100644 index 00000000000000..f7c29cdb5707d5 --- /dev/null +++ b/spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'rubocop_spec_helper' + +require_relative '../../../../../rubocop/cop/rspec/factory_bot/avoid_create_in_factories' + +RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AvoidCreateInFactories do + shared_examples 'an offensive factory call' do |namespace| + %i[create create_list].each do |forbidden_method| + namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:ci_job_artifact, :archive)" + + it "registers an offence for multiple #{namespaced_forbidden_method} calls" do + expect_offense(<<-RUBY) + FactoryBot.define do + factory :ci_build, class: 'Ci::Build', parent: :ci_processable do + trait :artifacts do + after(:create) do |build| + #{namespaced_forbidden_method} + #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories + #{namespaced_forbidden_method} + #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories + end + end + end + end + RUBY + end + + it "registers an offense for #{namespaced_forbidden_method} when is a send node" do + expect_offense(<<-RUBY) + FactoryBot.define do + factory :ci_build, class: 'Ci::Build', parent: :ci_processable do + trait :artifacts do + after(:create) do |build| + #{namespaced_forbidden_method} + #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories + end + end + end + end + RUBY + end + + it "registers an offense for #{namespaced_forbidden_method} when is assigned" do + expect_offense(<<-RUBY) + FactoryBot.define do + factory :ci_build, class: 'Ci::Build', parent: :ci_processable do + trait :artifacts do + after(:create) do |build| + ci_build = #{namespaced_forbidden_method} + #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories + + ci_build + end + end + end + end + RUBY + end + end + end + + it_behaves_like 'an offensive factory call', '' + it_behaves_like 'an offensive factory call', 'FactoryBot.' + it_behaves_like 'an offensive factory call', '::FactoryBot.' +end -- GitLab From c02ce20a8fd94890a811122a4c6b617251b90fb9 Mon Sep 17 00:00:00 2001 From: Alina Mihaila Date: Wed, 9 Nov 2022 18:17:59 +0200 Subject: [PATCH 2/6] Apply suggestions from maintainer --- .rubocop.yml | 2 +- ...factories.yml => strategy_in_callback.yml} | 14 +++- .../factory_bot/avoid_create_in_factories.rb | 64 ------------------- .../rspec/factory_bot/strategy_in_callback.rb | 45 +++++++++++++ ...s_spec.rb => strategy_in_callback_spec.rb} | 11 +++- 5 files changed, 67 insertions(+), 69 deletions(-) rename .rubocop_todo/rspec/factory_bot/{avoid_create_in_factories.yml => strategy_in_callback.yml} (51%) delete mode 100644 rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb create mode 100644 rubocop/cop/rspec/factory_bot/strategy_in_callback.rb rename spec/rubocop/cop/rspec/factory_bot/{avoid_create_in_factories_spec.rb => strategy_in_callback_spec.rb} (83%) diff --git a/.rubocop.yml b/.rubocop.yml index 6b158bcc72e977..6eb3612b0b3ef4 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -483,7 +483,7 @@ RSpec/FactoryBot/AvoidCreate: - 'ee/spec/presenters/**/*.rb' - 'ee/spec/serializers/**/*.rb' -RSpec/FactoryBot/AvoidCreateInFactories: +RSpec/FactoryBot/StrategyInCallback: Enabled: true Include: - 'spec/factories/*.rb' diff --git a/.rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml b/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml similarity index 51% rename from .rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml rename to .rubocop_todo/rspec/factory_bot/strategy_in_callback.yml index b19b4d6835b4e6..e66d7a88c37db3 100644 --- a/.rubocop_todo/rspec/factory_bot/avoid_create_in_factories.yml +++ b/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml @@ -1,19 +1,31 @@ --- -RSpec/FactoryBot/AvoidCreateInFactories: +RSpec/FactoryBot/StrategyInCallback: Details: grace period Exclude: + - 'ee/spec/factories/dast_scanner_profiles.rb' + - 'ee/spec/factories/description_version.rb' + - 'ee/spec/factories/epic_issues.rb' + - 'ee/spec/factories/geo_nodes.rb' - 'ee/spec/factories/groups.rb' - 'ee/spec/factories/merge_requests.rb' - 'ee/spec/factories/namespaces.rb' - 'ee/spec/factories/projects.rb' + - 'ee/spec/factories/protected_environments.rb' - 'ee/spec/factories/users.rb' - 'ee/spec/factories/vulnerabilities.rb' - 'ee/spec/factories/work_items.rb' + - 'spec/factories/commits.rb' + - 'spec/factories/container_expiration_policies.rb' - 'spec/factories/environments.rb' + - 'spec/factories/group_members.rb' - 'spec/factories/groups.rb' + - 'spec/factories/integrations.rb' - 'spec/factories/issues.rb' - 'spec/factories/merge_requests.rb' - 'spec/factories/namespaces.rb' + - 'spec/factories/pool_repositories.rb' + - 'spec/factories/project_members.rb' - 'spec/factories/projects.rb' - 'spec/factories/releases.rb' + - 'spec/factories/resource_label_events.rb' - 'spec/factories/users.rb' diff --git a/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb b/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb deleted file mode 100644 index 553a2fc7527cb0..00000000000000 --- a/rubocop/cop/rspec/factory_bot/avoid_create_in_factories.rb +++ /dev/null @@ -1,64 +0,0 @@ -# frozen_string_literal: true - -require 'rubocop-rspec' - -module RuboCop - module Cop - module RSpec - module FactoryBot - class AvoidCreateInFactories < RuboCop::Cop::Base - MSG = 'Prefer inline `association` over `%{type}`. ' \ - 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories' - - FORBIDDEN_METHODS = %i[create create_list].freeze - - def_node_matcher :send_node?, <<~PATTERN - send - PATTERN - - def_node_matcher :assign_node?, <<~PATTERN - lvasgn - PATTERN - - def_node_matcher :assign_node, <<~PATTERN - (lvasgn $...) - PATTERN - - def_node_matcher :forbidden_factory_usage, <<~PATTERN - ( - send - { nil? (const { nil? (cbase) } :FactoryBot) } - ${ #{FORBIDDEN_METHODS.map(&:inspect).join(' ')} } - (sym _) - ... - ) - PATTERN - - def on_block(node) - block_body = node.body - - return unless block_body - return unless node.method_name == :after - return unless node.send_node.arguments.first.value == :create - - if send_node?(block_body) - check_node(block_body) - else - block_body.children.each { |child_node| check_node(child_node) } - end - end - - private - - def check_node(node) - node = assign_node(node)[1] if assign_node?(node) - - forbidden_method_name = forbidden_factory_usage(node) - - add_offense(node, message: format(MSG, type: forbidden_method_name)) if forbidden_method_name.present? - end - end - end - end - end -end diff --git a/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb b/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb new file mode 100644 index 00000000000000..3153d54887ec9a --- /dev/null +++ b/rubocop/cop/rspec/factory_bot/strategy_in_callback.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'rubocop-rspec' + +module RuboCop + module Cop + module RSpec + module FactoryBot + class StrategyInCallback < RuboCop::Cop::Base + include RuboCop::RSpec::FactoryBot::Language + + MSG = 'Prefer inline `association` over `%{type}`. ' \ + 'See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories' + + FORBIDDEN_METHODS = %i[build build_list build_stubbed build_stubbed_list create create_list].freeze + + def_node_matcher :forbidden_factory_usage, <<~PATTERN + (block + (send nil? { :after :before } (sym _strategy)) + _args + ` # in all descandents + (send + { nil? #factory_bot? } + ${ #{FORBIDDEN_METHODS.map(&:inspect).join(' ')} } + (sym _factory_name) + ... + ) + ) + PATTERN + + RESTRICT_ON_SEND = FORBIDDEN_METHODS + + def on_send(node) + parent = node.each_ancestor(:block).first + + strategy = forbidden_factory_usage(parent) + return unless strategy + + add_offense(node, message: format(MSG, type: strategy)) + end + end + end + end + end +end diff --git a/spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb b/spec/rubocop/cop/rspec/factory_bot/strategy_in_callback_spec.rb similarity index 83% rename from spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb rename to spec/rubocop/cop/rspec/factory_bot/strategy_in_callback_spec.rb index f7c29cdb5707d5..9dcfebc7c1dae7 100644 --- a/spec/rubocop/cop/rspec/factory_bot/avoid_create_in_factories_spec.rb +++ b/spec/rubocop/cop/rspec/factory_bot/strategy_in_callback_spec.rb @@ -2,11 +2,11 @@ require 'rubocop_spec_helper' -require_relative '../../../../../rubocop/cop/rspec/factory_bot/avoid_create_in_factories' +require_relative '../../../../../rubocop/cop/rspec/factory_bot/strategy_in_callback' -RSpec.describe RuboCop::Cop::RSpec::FactoryBot::AvoidCreateInFactories do +RSpec.describe RuboCop::Cop::RSpec::FactoryBot::StrategyInCallback do shared_examples 'an offensive factory call' do |namespace| - %i[create create_list].each do |forbidden_method| + described_class::FORBIDDEN_METHODS.each do |forbidden_method| namespaced_forbidden_method = "#{namespace}#{forbidden_method}(:ci_job_artifact, :archive)" it "registers an offence for multiple #{namespaced_forbidden_method} calls" do @@ -14,6 +14,11 @@ FactoryBot.define do factory :ci_build, class: 'Ci::Build', parent: :ci_processable do trait :artifacts do + before(:create) do + #{namespaced_forbidden_method} + #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories + end + after(:create) do |build| #{namespaced_forbidden_method} #{'^' * namespaced_forbidden_method.size} Prefer inline `association` over `#{forbidden_method}`. See https://docs.gitlab.com/ee/development/testing_guide/best_practices.html#factories -- GitLab From 0510d8c265c87d2898c236b26c9b7cd611c910bb Mon Sep 17 00:00:00 2001 From: Alina Mihaila Date: Thu, 10 Nov 2022 10:45:26 +0200 Subject: [PATCH 3/6] Revert trace_artifact factory change --- spec/factories/ci/builds.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/factories/ci/builds.rb b/spec/factories/ci/builds.rb index 4013a411fac291..8396ef480c70d2 100644 --- a/spec/factories/ci/builds.rb +++ b/spec/factories/ci/builds.rb @@ -344,7 +344,9 @@ end trait :trace_artifact do - job_artifacts_trace { association(:ci_job_artifact, :trace) } + after(:create) do |build, evaluator| + create(:ci_job_artifact, :trace, job: build) + end end trait :unarchived_trace_artifact do -- GitLab From 58bd91e0bf206527a64776a8b950611a662d1a74 Mon Sep 17 00:00:00 2001 From: Alina Mihaila Date: Thu, 10 Nov 2022 12:42:42 +0200 Subject: [PATCH 4/6] Update factories docs --- .rubocop.yml | 4 ++-- .../factory_bot/strategy_in_callback.yml | 23 +++++++++++++++++++ .../testing_guide/best_practices.md | 2 ++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 6eb3612b0b3ef4..cedf6d2f56e78b 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -486,8 +486,8 @@ RSpec/FactoryBot/AvoidCreate: RSpec/FactoryBot/StrategyInCallback: Enabled: true Include: - - 'spec/factories/*.rb' - - 'ee/spec/factories/*.rb' + - 'spec/factories/**/*.rb' + - 'ee/spec/factories/**/*.rb' Cop/IncludeSidekiqWorker: Enabled: true diff --git a/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml b/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml index e66d7a88c37db3..456eca57804401 100644 --- a/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml +++ b/.rubocop_todo/rspec/factory_bot/strategy_in_callback.yml @@ -2,9 +2,13 @@ RSpec/FactoryBot/StrategyInCallback: Details: grace period Exclude: + - 'ee/spec/factories/ci/builds.rb' + - 'ee/spec/factories/ci/pipelines.rb' - 'ee/spec/factories/dast_scanner_profiles.rb' - 'ee/spec/factories/description_version.rb' + - 'ee/spec/factories/elastic/reindexing_tasks.rb' - 'ee/spec/factories/epic_issues.rb' + - 'ee/spec/factories/geo/design_registry.rb' - 'ee/spec/factories/geo_nodes.rb' - 'ee/spec/factories/groups.rb' - 'ee/spec/factories/merge_requests.rb' @@ -13,19 +17,38 @@ RSpec/FactoryBot/StrategyInCallback: - 'ee/spec/factories/protected_environments.rb' - 'ee/spec/factories/users.rb' - 'ee/spec/factories/vulnerabilities.rb' + - 'ee/spec/factories/vulnerabilities/external_issue_links.rb' + - 'ee/spec/factories/vulnerabilities/findings.rb' + - 'ee/spec/factories/vulnerabilities/issue_links.rb' - 'ee/spec/factories/work_items.rb' + - 'spec/factories/alert_management/alerts.rb' + - 'spec/factories/ci/builds.rb' + - 'spec/factories/ci/pipelines.rb' + - 'spec/factories/ci/processable.rb' + - 'spec/factories/ci/runner_namespaces.rb' + - 'spec/factories/ci/runner_projects.rb' + - 'spec/factories/ci/runners.rb' + - 'spec/factories/clusters/clusters.rb' - 'spec/factories/commits.rb' - 'spec/factories/container_expiration_policies.rb' + - 'spec/factories/design_management/designs.rb' + - 'spec/factories/design_management/versions.rb' - 'spec/factories/environments.rb' - 'spec/factories/group_members.rb' - 'spec/factories/groups.rb' - 'spec/factories/integrations.rb' - 'spec/factories/issues.rb' - 'spec/factories/merge_requests.rb' + - 'spec/factories/ml/candidates.rb' - 'spec/factories/namespaces.rb' + - 'spec/factories/packages/dependency_links.rb' + - 'spec/factories/packages/package_files.rb' + - 'spec/factories/packages/packages.rb' - 'spec/factories/pool_repositories.rb' - 'spec/factories/project_members.rb' - 'spec/factories/projects.rb' - 'spec/factories/releases.rb' - 'spec/factories/resource_label_events.rb' + - 'spec/factories/terraform/state.rb' - 'spec/factories/users.rb' + - 'spec/factories/work_items/parent_links.rb' diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 61f60567418d83..84567d614afa44 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -1444,6 +1444,8 @@ GitLab uses [factory_bot](https://github.com/thoughtbot/factory_bot) as a test f or [explicit](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#explicit-definition) association definitions instead of using `create` / `build` for association setup. See [issue #262624](https://gitlab.com/gitlab-org/gitlab/-/issues/262624) for further context. +- Prefer inline [associations](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition) instead of + using `create` / `build`. - Factories don't have to be limited to `ActiveRecord` objects. [See example](https://gitlab.com/gitlab-org/gitlab-foss/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). - Factories and their traits should produce valid objects that are [verified by specs](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/models/factories_spec.rb). -- GitLab From 3a6224efea4454830419ef0af71c77a0201688df Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Thu, 10 Nov 2022 11:56:45 +0000 Subject: [PATCH 5/6] Apply 1 suggestion(s) to 1 file(s) --- doc/development/testing_guide/best_practices.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 84567d614afa44..04e42c41f67e06 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -1440,9 +1440,10 @@ GitLab uses [factory_bot](https://github.com/thoughtbot/factory_bot) as a test f resulting record to pass validation. - When instantiating from a factory, don't supply attributes that aren't required by the test. -- Prefer [implicit](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#implicit-definition) - or [explicit](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#explicit-definition) - association definitions instead of using `create` / `build` for association setup. +- Prefer [implicit](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#implicit-definition), + [explicit](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#explicit-definition), or + [inline](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition) associations + over `create` / `build` for association setup in callbacks. See [issue #262624](https://gitlab.com/gitlab-org/gitlab/-/issues/262624) for further context. - Prefer inline [associations](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition) instead of using `create` / `build`. -- GitLab From d80265dc96a259959e638277b444d1e16f2889f1 Mon Sep 17 00:00:00 2001 From: Peter Leitzen Date: Thu, 10 Nov 2022 13:45:00 +0000 Subject: [PATCH 6/6] Apply 1 suggestion(s) to 1 file(s) --- doc/development/testing_guide/best_practices.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/development/testing_guide/best_practices.md b/doc/development/testing_guide/best_practices.md index 04e42c41f67e06..58f6b02c1f1c70 100644 --- a/doc/development/testing_guide/best_practices.md +++ b/doc/development/testing_guide/best_practices.md @@ -1445,8 +1445,6 @@ GitLab uses [factory_bot](https://github.com/thoughtbot/factory_bot) as a test f [inline](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition) associations over `create` / `build` for association setup in callbacks. See [issue #262624](https://gitlab.com/gitlab-org/gitlab/-/issues/262624) for further context. -- Prefer inline [associations](https://github.com/thoughtbot/factory_bot/blob/master/GETTING_STARTED.md#inline-definition) instead of - using `create` / `build`. - Factories don't have to be limited to `ActiveRecord` objects. [See example](https://gitlab.com/gitlab-org/gitlab-foss/commit/0b8cefd3b2385a21cfed779bd659978c0402766d). - Factories and their traits should produce valid objects that are [verified by specs](https://gitlab.com/gitlab-org/gitlab/-/blob/master/spec/models/factories_spec.rb). -- GitLab