Skip to content
代码片段 群组 项目
未验证 提交 be166074 编辑于 作者: Joe Woodward's avatar Joe Woodward 提交者: GitLab
浏览文件

Match namespaced factories in Migration/EnsureFactoryForTable Cop

Before this change we only matched tables against their file name e.g.

```ruby
create_table :projects_branch_rules_squash_options do
```

Would require a factory stored in:

`spec/factories/projects_branch_rules_squash_options.rb`

However, we often use namespacing to retain some structure within the
codebase. This change allows us to specify a factory in:

`spec/factories/projects/branch_rules/squash_options.rb`
上级 1730cfb2
No related branches found
No related tags found
无相关合并请求
显示
53 个添加45 个删除
...@@ -5,26 +5,16 @@ Migration/EnsureFactoryForTable: ...@@ -5,26 +5,16 @@ Migration/EnsureFactoryForTable:
- 'db/migrate/20230926092944_add_approval_group_rules_groups.rb' - 'db/migrate/20230926092944_add_approval_group_rules_groups.rb'
- 'db/migrate/20230926093004_add_approval_group_rules_users.rb' - 'db/migrate/20230926093004_add_approval_group_rules_users.rb'
- 'db/migrate/20230926093025_add_approval_group_rules_protected_branches.rb' - 'db/migrate/20230926093025_add_approval_group_rules_protected_branches.rb'
- 'db/migrate/20231017095738_create_activity_pub_releases_subscriptions.rb'
- 'db/migrate/20231017134349_create_ml_model_metadata.rb'
- 'db/migrate/20231020095624_create_audit_events_streaming_http_group_namespace_filters.rb' - 'db/migrate/20231020095624_create_audit_events_streaming_http_group_namespace_filters.rb'
- 'db/migrate/20231025123238_create_compliance_framework_security_policies.rb'
- 'db/migrate/20231031164724_create_sbom_occurrences_vulnerabilities.rb' - 'db/migrate/20231031164724_create_sbom_occurrences_vulnerabilities.rb'
- 'db/migrate/20231102142553_add_zoekt_nodes.rb'
- 'db/migrate/20231107140642_create_audit_events_instance_amazon_s3_configurations.rb'
- 'db/migrate/20231110044738_create_abuse_report_assignees_table.rb' - 'db/migrate/20231110044738_create_abuse_report_assignees_table.rb'
- 'db/migrate/20231115064007_create_audit_events_streaming_http_instance_namespace_filters.rb' - 'db/migrate/20231115064007_create_audit_events_streaming_http_instance_namespace_filters.rb'
- 'db/migrate/20231124191759_add_catalog_resource_sync_events_table.rb' - 'db/migrate/20231124191759_add_catalog_resource_sync_events_table.rb'
- 'db/migrate/20231207150738_add_work_item_dates_sources.rb' - 'db/migrate/20231207150738_add_work_item_dates_sources.rb'
- 'db/migrate/20231215192706_create_ml_model_version_metadata.rb'
- 'db/migrate/20231220113459_add_work_item_color.rb' - 'db/migrate/20231220113459_add_work_item_color.rb'
- 'db/migrate/20231221033539_create_sbom_source_packages_table.rb'
- 'db/migrate/20231221113948_create_cloud_connector_access.rb'
- 'db/migrate/20240108123023_create_ai_agents_table.rb' - 'db/migrate/20240108123023_create_ai_agents_table.rb'
- 'db/migrate/20240108123115_create_ai_agent_versions_table.rb' - 'db/migrate/20240108123115_create_ai_agent_versions_table.rb'
- 'db/migrate/20240112124030_create_audit_events_group_external_streaming_destinations.rb' - 'db/migrate/20240112124030_create_audit_events_group_external_streaming_destinations.rb'
- 'db/migrate/20240115185037_create_packages_terraform_module_metadata.rb'
- 'db/migrate/20240122165824_create_ci_job_token_group_scope_links.rb'
- 'db/migrate/20240130162148_create_audit_events_instance_external_streaming_destinations.rb' - 'db/migrate/20240130162148_create_audit_events_instance_external_streaming_destinations.rb'
- 'db/migrate/20240131052824_create_catalog_verified_namespaces.rb' - 'db/migrate/20240131052824_create_catalog_verified_namespaces.rb'
- 'db/migrate/20240201112236_create_group_saved_replies_table.rb' - 'db/migrate/20240201112236_create_group_saved_replies_table.rb'
...@@ -34,7 +24,3 @@ Migration/EnsureFactoryForTable: ...@@ -34,7 +24,3 @@ Migration/EnsureFactoryForTable:
- 'db/migrate/20240304184128_create_ci_build_names_table.rb' - 'db/migrate/20240304184128_create_ci_build_names_table.rb'
- 'db/migrate/20240306121653_create_relation_import_tracker.rb' - 'db/migrate/20240306121653_create_relation_import_tracker.rb'
- 'db/migrate/20240404192955_create_early_access_program_tracking_events.rb' - 'db/migrate/20240404192955_create_early_access_program_tracking_events.rb'
- 'db/migrate/20240419082037_create_ai_self_hosted_models.rb'
- 'db/migrate/20240423064716_create_ci_build_execution_config.rb'
- 'db/migrate/20240430110033_create_ai_feature_settings.rb'
- 'db/migrate/20241127092714_create_container_registry_protection_tag_rules.rb'
...@@ -4,7 +4,7 @@ class CreateDuoWorkflowsTables < Gitlab::Database::Migration[2.2] ...@@ -4,7 +4,7 @@ class CreateDuoWorkflowsTables < Gitlab::Database::Migration[2.2]
milestone '17.2' milestone '17.2'
def change def change
create_table :duo_workflows_workflows do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 create_table :duo_workflows_workflows do |t| # rubocop:disable Migration/EnsureFactoryForTable, Lint/RedundantCopDisableDirective -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630
t.bigint :user_id, null: false, index: true t.bigint :user_id, null: false, index: true
t.bigint :project_id, null: false, index: true t.bigint :project_id, null: false, index: true
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
class CreateDependencyListExportPartsTable < Gitlab::Database::Migration[2.2] class CreateDependencyListExportPartsTable < Gitlab::Database::Migration[2.2]
milestone '17.2' milestone '17.2'
# rubocop:disable Migration/EnsureFactoryForTable -- False Positive
def change def change
create_table :dependency_list_export_parts do |t| create_table :dependency_list_export_parts do |t|
t.references :dependency_list_export, foreign_key: { on_delete: :cascade }, null: false, index: true t.references :dependency_list_export, foreign_key: { on_delete: :cascade }, null: false, index: true
...@@ -15,5 +14,4 @@ def change ...@@ -15,5 +14,4 @@ def change
t.text :file, limit: 255 t.text :file, limit: 255
end end
end end
# rubocop:enable Migration/EnsureFactoryForTable
end end
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
class CreatePCiBuildTraceMetadata < Gitlab::Database::Migration[2.2] class CreatePCiBuildTraceMetadata < Gitlab::Database::Migration[2.2]
milestone '17.4' milestone '17.4'
# rubocop:disable Migration/EnsureFactoryForTable -- No factory needed
def change def change
create_table(:p_ci_build_trace_metadata, primary_key: [:build_id, :partition_id], create_table(:p_ci_build_trace_metadata, primary_key: [:build_id, :partition_id],
options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t| options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t|
...@@ -19,5 +18,4 @@ def change ...@@ -19,5 +18,4 @@ def change
t.index :trace_artifact_id t.index :trace_artifact_id
end end
end end
# rubocop:enable Migration/EnsureFactoryForTable -- No factory needed
end end
# frozen_string_literal: true # frozen_string_literal: true
# rubocop:disable Migration/EnsureFactoryForTable -- False positive
class CreateSystemAccessInstanceMicrosoftApplications < Gitlab::Database::Migration[2.2] class CreateSystemAccessInstanceMicrosoftApplications < Gitlab::Database::Migration[2.2]
milestone '17.5' milestone '17.5'
...@@ -17,5 +16,3 @@ def change ...@@ -17,5 +16,3 @@ def change
end end
end end
end end
# rubocop:enable Migration/EnsureFactoryForTable
# frozen_string_literal: true # frozen_string_literal: true
# rubocop:disable Migration/EnsureFactoryForTable -- False positive
class CreateSystemAccessInstanceMicrosoftGraphAccessTokens < Gitlab::Database::Migration[2.2] class CreateSystemAccessInstanceMicrosoftGraphAccessTokens < Gitlab::Database::Migration[2.2]
milestone '17.5' milestone '17.5'
...@@ -16,5 +15,3 @@ def change ...@@ -16,5 +15,3 @@ def change
end end
end end
end end
# rubocop:enable Migration/EnsureFactoryForTable
...@@ -4,7 +4,7 @@ class CreateInstanceIntegrationsTable < Gitlab::Database::Migration[2.2] ...@@ -4,7 +4,7 @@ class CreateInstanceIntegrationsTable < Gitlab::Database::Migration[2.2]
milestone '17.4' milestone '17.4'
def up def up
create_table :instance_integrations, id: :bigserial do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 create_table :instance_integrations, id: :bigserial do |t|
t.timestamps_with_timezone null: false t.timestamps_with_timezone null: false
t.integer :comment_detail t.integer :comment_detail
t.boolean :active, default: false, null: false t.boolean :active, default: false, null: false
......
...@@ -5,7 +5,7 @@ class CreateCiJobArtifactReports < Gitlab::Database::Migration[2.2] ...@@ -5,7 +5,7 @@ class CreateCiJobArtifactReports < Gitlab::Database::Migration[2.2]
disable_ddl_transaction! disable_ddl_transaction!
def up def up
create_table(:p_ci_job_artifact_reports, # rubocop:disable Migration/EnsureFactoryForTable -- Factory exists create_table(:p_ci_job_artifact_reports,
primary_key: [:job_artifact_id, :partition_id], primary_key: [:job_artifact_id, :partition_id],
options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t| options: 'PARTITION BY LIST (partition_id)', if_not_exists: true) do |t|
t.bigint :job_artifact_id, null: false t.bigint :job_artifact_id, null: false
......
...@@ -6,7 +6,7 @@ class CreatePackagesConanRecipeRevisions < Gitlab::Database::Migration[2.2] ...@@ -6,7 +6,7 @@ class CreatePackagesConanRecipeRevisions < Gitlab::Database::Migration[2.2]
INDEX_PACKAGE_ID_REVISION = 'idx_on_packages_conan_recipe_revisions_package_id_revision' INDEX_PACKAGE_ID_REVISION = 'idx_on_packages_conan_recipe_revisions_package_id_revision'
def up def up
create_table :packages_conan_recipe_revisions do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 create_table :packages_conan_recipe_revisions do |t|
t.bigint :package_id, null: false t.bigint :package_id, null: false
t.bigint :project_id, null: false t.bigint :project_id, null: false
t.timestamps_with_timezone null: false t.timestamps_with_timezone null: false
......
...@@ -7,7 +7,7 @@ class CreatePackagesConanPackageReferences < Gitlab::Database::Migration[2.2] ...@@ -7,7 +7,7 @@ class CreatePackagesConanPackageReferences < Gitlab::Database::Migration[2.2]
CONSTRAINT_NAME = 'chk_conan_references_info_length' CONSTRAINT_NAME = 'chk_conan_references_info_length'
def up def up
create_table :packages_conan_package_references do |t| # rubocop:disable Migration/EnsureFactoryForTable -- https://gitlab.com/gitlab-org/gitlab/-/issues/468630 create_table :packages_conan_package_references do |t|
t.bigint :package_id, null: false t.bigint :package_id, null: false
t.bigint :project_id, null: false t.bigint :project_id, null: false
t.bigint :recipe_revision_id t.bigint :recipe_revision_id
......
...@@ -6,7 +6,7 @@ class CreateCiJobTokenAuthorizations < Gitlab::Database::Migration[2.2] ...@@ -6,7 +6,7 @@ class CreateCiJobTokenAuthorizations < Gitlab::Database::Migration[2.2]
INDEX_NAME = 'idx_ci_job_token_authorizations_on_accessed_and_origin_project' INDEX_NAME = 'idx_ci_job_token_authorizations_on_accessed_and_origin_project'
def change def change
create_table(:ci_job_token_authorizations, if_not_exists: true) do |t| # rubocop:disable Migration/EnsureFactoryForTable -- factory at ci/job_token/authorizations.rb create_table(:ci_job_token_authorizations, if_not_exists: true) do |t|
t.bigint :accessed_project_id, null: false t.bigint :accessed_project_id, null: false
t.bigint :origin_project_id, null: false, index: true t.bigint :origin_project_id, null: false, index: true
t.datetime_with_timezone :last_authorized_at, null: false t.datetime_with_timezone :last_authorized_at, null: false
......
...@@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftApplications < Gitlab::Database::Migration ...@@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftApplications < Gitlab::Database::Migration
milestone '17.7' milestone '17.7'
def change def change
create_table :system_access_group_microsoft_applications do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive, factory name is prefixed with system_access create_table :system_access_group_microsoft_applications do |t|
t.timestamps_with_timezone null: false t.timestamps_with_timezone null: false
t.references :group, foreign_key: { to_table: :namespaces, on_delete: :cascade }, null: false t.references :group, foreign_key: { to_table: :namespaces, on_delete: :cascade }, null: false
t.bigint :temp_source_id, index: { unique: true, name: 'index_group_microsoft_applications_on_temp_source_id' }, t.bigint :temp_source_id, index: { unique: true, name: 'index_group_microsoft_applications_on_temp_source_id' },
......
...@@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftGraphAccessTokens < Gitlab::Database::Migr ...@@ -5,7 +5,7 @@ class CreateSystemAccessGroupMicrosoftGraphAccessTokens < Gitlab::Database::Migr
milestone '17.7' milestone '17.7'
def change def change
create_table :system_access_group_microsoft_graph_access_tokens do |t| # rubocop:disable Migration/EnsureFactoryForTable -- False positive, factory name is prefixed with system_access create_table :system_access_group_microsoft_graph_access_tokens do |t|
t.timestamps_with_timezone null: false t.timestamps_with_timezone null: false
t.references :system_access_group_microsoft_application, t.references :system_access_group_microsoft_application,
index: { name: 'unique_index_group_ms_access_tokens_on_ms_app_id', unique: true } index: { name: 'unique_index_group_ms_access_tokens_on_ms_app_id', unique: true }
......
...@@ -47,7 +47,8 @@ def on_send(node) ...@@ -47,7 +47,8 @@ def on_send(node)
return if !ee? && disabled_comment_absent? return if !ee? && disabled_comment_absent?
table_definition(node) do |table_name_node, table_name| table_definition(node) do |table_name_node, table_name|
unless factory?(table_name.to_s) # Partioned tables are prefix with `p_`.
unless factory?(table_name.to_s.delete_prefix('p_'))
msg = format(MSG, name: table_name) msg = format(MSG, name: table_name)
add_offense(table_name_node, message: msg) add_offense(table_name_node, message: msg)
end end
...@@ -57,13 +58,13 @@ def on_send(node) ...@@ -57,13 +58,13 @@ def on_send(node)
private private
def factory?(table_name) def factory?(table_name)
end_with = "/#{table_name}.rb" self.class.factories.any? { |name| name.end_with?(table_name) }
self.class.factories.any? { |path| path.end_with?(end_with) }
end end
def self.factories def self.factories
@factories ||= Dir.glob("{,ee/,jh/}spec/factories/**/*.rb") @factories ||= Dir.glob("{,ee/,jh/}spec/factories/**/*.rb").map do |factory|
factory.gsub(%r{^(ee/|jh/|)spec/factories/}, '').delete_suffix('.rb').tr('/', '_')
end
end end
def disabled_comment_absent? def disabled_comment_absent?
......
...@@ -83,22 +83,53 @@ ...@@ -83,22 +83,53 @@
end end
context 'with matching factories' do context 'with matching factories' do
let(:factories) { ['spec/factories/users.rb'] } context 'with regular table' do
let(:factories) { ['users'] }
it 'does not register an offense when a table has a corresponding factory' do it 'does not register an offense when a table has a corresponding factory' do
expect_no_offenses(<<~RUBY) expect_no_offenses(<<~RUBY)
create_table :users do |t| create_table :users do |t|
t.string :name t.string :name
t.timestamps t.timestamps
end
RUBY
end
end
context 'with partitioned table' do
let(:factories) { ['users'] }
it 'does not register an offense when a table has a corresponding factory' do
expect_no_offenses(<<~RUBY)
create_table :p_users do |t|
t.string :name
t.timestamps
end
RUBY
end end
RUBY
end end
end end
end end
describe '.factories' do describe '.factories' do
let(:table_names) { %w[unnested_ce nested_ce_factory unnested_ee nested_ee_factory unnested_jh nested_jh_factory] }
let(:factories) do
%w[
spec/factories/unnested_ce.rb
spec/factories/nested/ce/factory.rb
ee/spec/factories/unnested_ee.rb
ee/spec/factories/nested/ee_factory.rb
jh/spec/factories/unnested_jh.rb
jh/spec/factories/nested_jh/factory.rb
]
end
before do
allow(Dir).to receive(:glob).and_return(factories)
end
subject { described_class.factories } subject { described_class.factories }
it { is_expected.not_to be_empty } it { is_expected.to eq(table_names) }
end end
end end
0% 加载中 .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册