Add rubocop cop to prevent model usage in migration specs

.rubocop_cc.yml

@@ -11,6 +11,7 @@ plugins:
 require:
   - ./spec/linters/migration/add_constraint_name.rb
   - ./spec/linters/migration/include_string_size.rb
+  - ./spec/linters/migration/no_model_in_specs.rb
   - ./spec/linters/migration/require_primary_key.rb
   - ./spec/linters/migration/too_many_migration_runs.rb
   - ./spec/linters/match_requires_with_includes.rb
@@ -67,6 +68,12 @@ Migration/IncludeStringSize: # Exclude for old Migrations
   Exclude:
     - !ruby/regexp /db/migrations/201([0-9]).+\.rb$/
     - !ruby/regexp /db/migrations/202([0-2]).+\.rb$/
+Migration/NoModelInSpecs:
+  Include:
+    - spec/migrations/**/*
+  Exclude:
+    # Uses custom tmp_migrations_dir so schema is never rolled back
+    - spec/migrations/20190712210940_backfill_status_for_deployments_spec.rb
 Migration/RequirePrimaryKey: # Exclude for old Migrations
   Include:
     - db/migrations/**/*

spec/linters/migration/no_model_in_specs.rb

@@ -0,0 +1,25 @@
+module RuboCop
+  module Cop
+    module Migration
+      class NoModelInSpecs < RuboCop::Cop::Base
+        MSG = 'Do not use model classes in migration specs. ' \
+              'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \
+              'See spec/migrations/Readme.md for details.'.freeze
+
+        def on_send(node)
+          add_offense(node) if model_receiver?(node.receiver)
+        end
+
+        private
+
+        def model_receiver?(receiver)
+          return false unless receiver
+          return false unless receiver.const_type?
+
+          name = receiver.const_name.to_s
+          name.end_with?('Model') && name != 'Sequel::Model'
+        end
+      end
+    end
+  end
+end

spec/linters/migration/no_model_in_specs_spec.rb

@@ -0,0 +1,74 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/cop_helper'
+require 'rubocop/config'
+require 'linters/migration/no_model_in_specs'
+
+RSpec.describe RuboCop::Cop::Migration::NoModelInSpecs do
+  include CopHelper
+
+  subject(:cop) { described_class.new(RuboCop::Config.new({})) }
+
+  let(:message) do
+    'Do not use model classes in migration specs. ' \
+      'Use raw Sequel operations (e.g. db[:table].insert) instead. ' \
+      'See spec/migrations/Readme.md for details.'
+  end
+
+  it 'registers an offense for Model.make' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.make
+    RUBY
+
+    expect(result.size).to eq(1)
+    expect(result.map(&:message)).to eq([message])
+  end
+
+  it 'registers an offense for Model.make!' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.make!
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'registers an offense for Model.create' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::DeploymentModel.create(guid: 'test')
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'registers an offense for Model.where' do
+    result = inspect_source(<<~RUBY)
+      VCAP::CloudController::AppModel.where(guid: 'test').first
+    RUBY
+
+    expect(result.size).to eq(1)
+  end
+
+  it 'does not register an offense for raw Sequel inserts' do
+    result = inspect_source(<<~RUBY)
+      db[:apps].insert(guid: 'test', name: 'test-app')
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+
+  it 'does not register an offense for Sequel::Model' do
+    result = inspect_source(<<~RUBY)
+      Sequel::Model.db
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+
+  it 'does not register an offense for non-Model constants' do
+    result = inspect_source(<<~RUBY)
+      SecureRandom.uuid
+    RUBY
+
+    expect(result.size).to eq(0)
+  end
+end

spec/migrations/20251030100000_add_unique_constraint_to_sidecar_process_types_spec.rb

@@ -6,22 +6,27 @@
     let(:migration_filename) { '20251030100000_add_unique_constraint_to_sidecar_process_types.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
-  let!(:sidecar) { VCAP::CloudController::SidecarModel.make(app:) }
-  let!(:revision) { VCAP::CloudController::RevisionModel.make(app:) }
-  let!(:revision_sidecar) { VCAP::CloudController::RevisionSidecarModel.make(revision:) }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:sidecar_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
+  let(:revision_sidecar_guid) { SecureRandom.uuid }
 
   it 'removes duplicates, adds unique constraints, and is reversible' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:sidecars].insert(guid: sidecar_guid, name: 'test-sidecar', command: 'command', app_guid: app_guid)
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description')
+    db[:revision_sidecars].insert(guid: revision_sidecar_guid, name: 'test-sidecar', command: 'command', revision_guid: revision_guid)
+
     # =========================================================================================
     # SETUP: Create duplicate entries for both tables to test the de-duplication logic.
     # =========================================================================================
-    db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid)
-    db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid)
-    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(2)
+    db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid)
+    db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid)
+    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(2)
 
-    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid)
-    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid)
-    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(2)
+    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid)
+    db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid)
+    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(2)
 
     # =========================================================================================
     # UP MIGRATION: Run the migration to apply the unique constraints.
@@ -31,13 +36,13 @@
     # =========================================================================================
     # ASSERT UP MIGRATION: Verify that duplicates are removed and constraints are enforced.
     # =========================================================================================
-    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar.guid, type: 'web').count).to eq(1)
+    expect(db[:sidecar_process_types].where(sidecar_guid: sidecar_guid, type: 'web').count).to eq(1)
     expect(db.indexes(:sidecar_process_types)).to include(:sidecar_process_types_sidecar_guid_type_index)
-    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
-    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar.guid, type: 'worker').count).to eq(1)
+    expect(db[:revision_sidecar_process_types].where(revision_sidecar_guid: revision_sidecar_guid, type: 'worker').count).to eq(1)
     expect(db.indexes(:revision_sidecar_process_types)).to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index)
-    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # =========================================================================================
     # TEST IDEMPOTENCY: Running the migration again should not cause any errors.
@@ -53,9 +58,9 @@
     # ASSERT DOWN MIGRATION: Verify that constraints are removed and duplicates can be re-inserted.
     # =========================================================================================
     expect(db.indexes(:sidecar_process_types)).not_to include(:sidecar_process_types_sidecar_guid_type_index)
-    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar.guid, type: 'web', app_guid: app.guid, guid: SecureRandom.uuid) }.not_to raise_error
+    expect { db[:sidecar_process_types].insert(sidecar_guid: sidecar_guid, type: 'web', app_guid: app_guid, guid: SecureRandom.uuid) }.not_to raise_error
 
     expect(db.indexes(:revision_sidecar_process_types)).not_to include(:revision_sidecar_process_types_revision_sidecar_guid_type_index)
-    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar.guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error
+    expect { db[:revision_sidecar_process_types].insert(revision_sidecar_guid: revision_sidecar_guid, type: 'worker', guid: SecureRandom.uuid) }.not_to raise_error
   end
 end

spec/migrations/20260320141005_add_unique_constraint_to_revision_sidecars_spec.rb

@@ -5,22 +5,25 @@
     let(:migration_filename) { '20260320141005_add_unique_constraint_to_revision_sidecars.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
-  let!(:revision) { VCAP::CloudController::RevisionModel.make(:app) }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
 
   it 'remove duplicates, add constraint and revert migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description')
+
     # create duplicate entries
-    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid)
-    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid)
-    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(2)
+    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid)
+    db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid)
+    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision.guid).count).to eq(1)
+    expect(db[:revision_sidecars].where(name: 'app', revision_guid: revision_guid).count).to eq(1)
     expect(db.indexes(:revision_sidecars)).to include(:revision_sidecars_revision_guid_name_index)
-    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
     expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
@@ -30,6 +33,6 @@
 
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:revision_sidecars)).not_to include(:revision_sidecars_revision_guid_name_index)
-    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision.guid) }.not_to raise_error
+    expect { db[:revision_sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', revision_guid: revision_guid) }.not_to raise_error
   end
 end

spec/migrations/20260323092954_add_unique_constraint_to_sidecars_spec.rb

@@ -5,21 +5,23 @@
     let(:migration_filename) { '20260323092954_add_unique_constraint_to_sidecars.rb' }
   end
 
-  let!(:app) { VCAP::CloudController::AppModel.make }
+  let(:app_guid) { SecureRandom.uuid }
 
   it 'remove duplicates, add constraint and revert migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+
     # create duplicate entries
-    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid)
-    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid)
-    expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(2)
+    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid)
+    db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid)
+    expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:sidecars].where(name: 'app', app_guid: app.guid).count).to eq(1)
+    expect(db[:sidecars].where(name: 'app', app_guid: app_guid).count).to eq(1)
     expect(db.indexes(:sidecars)).to include(:sidecars_app_guid_name_index)
-    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.to raise_error(Sequel::UniqueConstraintViolation)
+    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
     expect { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }.not_to raise_error
@@ -29,6 +31,6 @@
 
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:sidecars)).not_to include(:sidecars_app_guid_name_index)
-    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app.guid) }.not_to raise_error
+    expect { db[:sidecars].insert(guid: SecureRandom.uuid, name: 'app', command: 'command', app_guid: app_guid) }.not_to raise_error
   end
 end

spec/migrations/20260323144429_add_unique_constraint_to_revision_process_commands_spec.rb

@@ -6,22 +6,26 @@
     let(:migration_filename) { '20260323144429_add_unique_constraint_to_revision_process_commands.rb' }
   end
 
-  let!(:revision) { VCAP::CloudController::RevisionModel.make }
+  let(:app_guid) { SecureRandom.uuid }
+  let(:revision_guid) { SecureRandom.uuid }
 
   it 'removes duplicates, adds constraint and reverts migration' do
+    db[:apps].insert(guid: app_guid, name: 'test-app')
+    db[:revisions].insert(guid: revision_guid, app_guid: app_guid, description: 'some description')
+
     # create duplicate entries
-    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
-    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
-    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(2)
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
+    db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
+    expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(2)
 
     # run the migration
     Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true)
 
     # verify duplicates are removed and constraint is enforced
-    expect(db[:revision_process_commands].where(revision_guid: revision.guid, process_type: 'worker').count).to eq(1)
+    expect(db[:revision_process_commands].where(revision_guid: revision_guid, process_type: 'worker').count).to eq(1)
     expect(db.indexes(:revision_process_commands)).to include(:revision_process_commands_revision_guid_process_type_index)
     expect do
-      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
     end.to raise_error(Sequel::UniqueConstraintViolation)
 
     # running the migration again should not cause any errors
@@ -33,7 +37,7 @@
     # verify constraint is removed and duplicates can be re-inserted
     expect(db.indexes(:revision_process_commands)).not_to include(:revision_process_commands_revision_guid_process_type_index)
     expect do
-      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision.guid, process_type: 'worker')
+      db[:revision_process_commands].insert(guid: SecureRandom.uuid, revision_guid: revision_guid, process_type: 'worker')
     end.not_to raise_error
   end
 end