Replace machinist with FactoryBot for test data setup (cloudfoundry#5121)

Per ADR 0015. machinist 1.0.6 has had no upstream activity since 2013
and machinist 2.0 is not a viable upgrade (no Sequel adapter, no Sham,
non-persisting make). FactoryBot is actively maintained, has trivial
Sequel integration, and provides equivalents for every pattern used
by the existing blueprints.

Convert ~11k call sites from Model.make / Model.make_unsaved to
create(:foo) / build(:foo), replace ~130 blueprints with factory
definitions under spec/support/factory_definitions/, and remove the
machinist gem along with spec/support/fakes/blueprints.rb and
spec/support/machinist_monkey_patch.rb.

Key technical decisions (see ADR 0015):

- Global to_create { |i| i.save; i.refresh } in
  spec/support/factories.rb. This matches machinist's Sequel adapter
  which both saved and refreshed; the refresh prevents stale-cache
  bugs whenever a factory's after-create hook mutates associations.

- Sham is preserved as a thin shim (spec/support/sham_shim.rb) that
  delegates Sham.<name> to FactoryBot.generate(:sham_<name>) sequences
  defined in spec/support/factories.rb. The shim mirrors the original
  Sham.define block 1:1 so existing call sites need no edits.

- Dynamic class -> factory-name conversion via
  klass.name.demodulize.underscore.to_sym is used in matchers and
  shared examples, so generic helpers continue to look up the right
  factory when given any model class.

- Named blueprints become traits: Foo.blueprint(:bar) turns into a
  trait :bar on the :foo factory; Foo.make(:bar, x: 1) becomes
  create(:foo, :bar, x: 1).

- build replaces make_unsaved for the (rare) cases that wanted an
  unsaved instance.

- :droplet_model only auto-sets itself as the app's current droplet
  when no app: override is supplied (set_as_current_droplet
  { app == :unset }), matching machinist's blueprint where the default
  app block only ran in that case. Specs that previously relied on the
  auto-set side effect when passing app: explicitly call
  app.update(droplet:) just as the pre-migration versions did.

- :revision_sidecar_process_type_model builds its parent with the
  :no_process_types trait so FactoryBot.lint does not collide with the
  parent's after-create web row on the (revision_sidecar_guid, type)
  unique constraint.

Author: philippthun

Gemfile

@@ -58,7 +58,7 @@ end
 
 group :test do
   gem 'codeclimate-test-reporter', '>= 1.0.8', require: false
-  gem 'machinist', '~> 1.0.6'
+  gem 'factory_bot', '~> 6.5'
   gem 'mock_redis'
   gem 'parallel_tests'
   gem 'rack-test'

Gemfile.lock

@@ -86,6 +86,8 @@ GEM
     erubi (1.13.1)
     excon (1.5.0)
       logger
+    factory_bot (6.6.0)
+      activesupport (>= 6.1.0)
     faraday (0.17.6)
       multipart-post (>= 1.2, < 3)
     ffi (1.17.4)
@@ -224,7 +226,6 @@ GEM
     loofah (2.25.1)
       crass (~> 1.0.2)
       nokogiri (>= 1.12.0)
-    machinist (1.0.6)
     method_source (1.1.0)
     mime-types (3.7.0)
       logger
@@ -562,6 +563,7 @@ DEPENDENCIES
   concurrent-ruby
   debug (~> 1.11)
   digest-xxhash
+  factory_bot (~> 6.5)
   fluent-logger
   fog-aliyun
   fog-aws
@@ -573,7 +575,6 @@ DEPENDENCIES
   json-diff
   json-schema
   listen
-  machinist (~> 1.0.6)
   mime-types (~> 3.7)
   mock_redis
   multipart-parser

lib/cloud_controller/console.rb

@@ -36,9 +36,8 @@
 
 if ENV['NEW_RELIC_ENV'] == 'development'
   $LOAD_PATH.unshift(File.expand_path('../../spec/support', __dir__))
-  require 'machinist/sequel'
-  require 'machinist/object'
-  require 'fakes/blueprints'
+  require 'factories'
+  require 'sham_shim'
 end
 
 module VCAP::CloudController

spec/acceptance/async_bindings_spec.rb

@@ -33,16 +33,16 @@ module VCAP::CloudController
       end
 
       context 'when a service instance is shared' do
-        let(:service_instance) { ManagedServiceInstance.make }
-        let(:target_space) { Space.make }
+        let(:service_instance) { create(:managed_service_instance) }
+        let(:target_space) { create(:space) }
 
         before do
           service_instance.add_shared_space(target_space)
         end
 
         context 'when there are bindings in the target space' do
-          let(:target_app) { AppModel.make(space: target_space) }
-          let!(:target_binding) { ServiceBinding.make(app: target_app, service_instance: service_instance) }
+          let(:target_app) { create(:app_model, space: target_space) }
+          let!(:target_binding) { create(:service_binding, app: target_app, service_instance: service_instance) }
 
           it 'issues an unbind and fails the instance deletion if the service instance is deleted recursively and accepts_incomplete is true' do
             delete("/v2/service_instances/#{service_instance.guid}", 'recursive=true&accepts_incomplete=true', admin_headers)
@@ -70,8 +70,8 @@ module VCAP::CloudController
 
           context 'and when there are bindings in the source space' do
             let(:source_space) { service_instance.space }
-            let(:source_app) { AppModel.make(space: source_space) }
-            let!(:source_binding) { ServiceBinding.make(app: source_app, service_instance: service_instance) }
+            let(:source_app) { create(:app_model, space: source_space) }
+            let!(:source_binding) { create(:service_binding, app: source_app, service_instance: service_instance) }
 
             it 'issues unbinds and fails the instance deletion if the service instance is deleted recursively and accepts_incomplete is true' do
               delete("/v2/service_instances/#{service_instance.guid}", 'recursive=true&accepts_incomplete=true', admin_headers)
@@ -121,15 +121,15 @@ module VCAP::CloudController
 
       context 'when DELETE /v3/apps/:guid is called' do
         context 'and multiple service bindings exist' do
-          let(:space) { Space.make }
-          let(:app_model) { VCAP::CloudController::AppModel.make(name: 'app_name', space: space) }
-          let(:package) { VCAP::CloudController::PackageModel.make(app: app_model) }
-          let!(:droplet) { VCAP::CloudController::DropletModel.make(package: package, app: app_model) }
-          let!(:process) { VCAP::CloudController::ProcessModel.make(app: app_model) }
-          let!(:deployment) { VCAP::CloudController::DeploymentModel.make(app: app_model) }
+          let(:space) { create(:space) }
+          let(:app_model) { create(:app_model, name: 'app_name', space: space) }
+          let(:package) { create(:package_model, app: app_model) }
+          let!(:droplet) { create(:droplet_model, package: package, app: app_model) }
+          let!(:process) { create(:process_model, app: app_model) }
+          let!(:deployment) { create(:deployment_model, app: app_model) }
 
-          let!(:service_binding1) { ServiceBinding.make(app: app_model, service_instance: ManagedServiceInstance.make(space:)) }
-          let!(:service_binding2) { ServiceBinding.make(app: app_model, service_instance: ManagedServiceInstance.make(space:)) }
+          let!(:service_binding1) { create(:service_binding, app: app_model, service_instance: create(:managed_service_instance, space:)) }
+          let!(:service_binding2) { create(:service_binding, app: app_model, service_instance: create(:managed_service_instance, space:)) }
 
           it 'returns a list of errors for the service bindings' do
             delete("/v3/apps/#{app_model.guid}", nil, admin_headers)
@@ -157,8 +157,8 @@ module VCAP::CloudController
         context 'and multiple service bindings exist' do
           let(:process) { ProcessModelFactory.make }
 
-          let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
-          let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
+          let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
+          let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
 
           it 'returns a concatenated error for the service bindings' do
             delete("/v2/apps/#{process.app.guid}", 'recursive=true', admin_headers)
@@ -180,8 +180,8 @@ module VCAP::CloudController
         context 'and multiple service bindings exist' do
           let(:process) { ProcessModelFactory.make }
 
-          let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
-          let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
+          let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
+          let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
 
           it 'returns a concatenated error for the service bindings' do
             delete("/v2/spaces/#{process.space.guid}", 'recursive=true', admin_headers)
@@ -203,8 +203,8 @@ module VCAP::CloudController
         context 'and multiple service bindings exist' do
           let(:process) { ProcessModelFactory.make }
 
-          let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
-          let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
+          let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
+          let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
 
           it 'returns a concatenated error for the service bindings' do
             delete("/v2/organizations/#{process.organization.guid}", 'recursive=true', admin_headers)
@@ -224,8 +224,8 @@ module VCAP::CloudController
 
       context 'when PUT /v2/service_instances/:guid is called and when an async binding operation is in progress' do
         let(:process) { ProcessModelFactory.make }
-        let(:service_binding) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
-        let!(:service_binding_operation) { ServiceBindingOperation.make(state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
+        let(:service_binding) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
+        let!(:service_binding_operation) { create(:service_binding_operation, state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
 
         let(:body) do
           {
@@ -270,8 +270,8 @@ module VCAP::CloudController
 
       context 'when DELETE /v2/service_instances/:guid is called and when an async binding operation is in progress' do
         let(:process) { ProcessModelFactory.make }
-        let(:service_binding) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
-        let!(:service_binding_operation) { ServiceBindingOperation.make(state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
+        let(:service_binding) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
+        let!(:service_binding_operation) { create(:service_binding_operation, state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
 
         context 'when the binding operation is create' do
           let(:operation_type) { 'create' }

spec/acceptance/broker_api_compatibility/broker_api_v2.13_spec.rb

@@ -227,7 +227,7 @@
       let(:catalog) { default_catalog(plan_updateable: true) }
 
       context 'service broker registration' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           setup_broker_with_user(user)
@@ -246,7 +246,7 @@
       end
 
       context 'service provision request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service(user:)
@@ -264,7 +264,7 @@
       end
 
       context 'service deprovision request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service(user:)
@@ -283,7 +283,7 @@
       end
 
       context 'service update request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service(user:)
@@ -302,7 +302,7 @@
       end
 
       context 'service binding request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service
@@ -322,7 +322,7 @@
       end
 
       context 'service unbind request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
         let(:async) { false }
 
         before do
@@ -367,7 +367,7 @@
       end
 
       context 'create service key request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service
@@ -386,7 +386,7 @@
       end
 
       context 'delete service key request' do
-        let(:user) { VCAP::CloudController::User.make }
+        let(:user) { create(:user) }
 
         before do
           provision_service
@@ -407,8 +407,8 @@
 
       context 'create route binding' do
         let(:catalog) { default_catalog(plan_updateable: true, requires: ['route_forwarding']) }
-        let(:user) { VCAP::CloudController::User.make }
-        let(:route) { VCAP::CloudController::Route.make(space: @space) }
+        let(:user) { create(:user) }
+        let(:route) { create(:route, space: @space) }
 
         before do
           provision_service
@@ -428,8 +428,8 @@
 
       context 'delete route binding' do
         let(:catalog) { default_catalog(plan_updateable: true, requires: ['route_forwarding']) }
-        let(:user) { VCAP::CloudController::User.make }
-        let(:route) { VCAP::CloudController::Route.make(space: @space) }
+        let(:user) { create(:user) }
+        let(:route) { create(:route, space: @space) }
 
         before do
           provision_service
@@ -449,9 +449,9 @@
       end
 
       context 'when multiple users operate on a service instance' do
-        let(:user_a) { VCAP::CloudController::User.make }
-        let(:user_b) { VCAP::CloudController::User.make }
-        let(:user_c) { VCAP::CloudController::User.make }
+        let(:user_a) { create(:user) }
+        let(:user_b) { create(:user) }
+        let(:user_c) { create(:user) }
 
         before do
           provision_service(user: user_a)
@@ -547,7 +547,7 @@
 
       context 'for bind route service' do
         let(:catalog) { default_catalog(requires: ['route_forwarding']) }
-        let(:route) { VCAP::CloudController::Route.make(space: @space) }
+        let(:route) { create(:route, space: @space) }
 
         before do
           provision_service

spec/acceptance/broker_api_compatibility/broker_api_v2.14_spec.rb

@@ -48,7 +48,7 @@
           end
 
           it 'sends the broker the X-Broker-Api-Originating-Identity header' do
-            user = VCAP::CloudController::User.make
+            user = create(:user)
 
             get("/v2/service_bindings/#{@binding_guid}/parameters",
                 {}.to_json,
@@ -249,7 +249,7 @@
     end
 
     describe 'update service dashboard url' do
-      let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+      let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
       let(:catalog) { default_catalog(plan_updateable: true) }
 
       before do

spec/acceptance/broker_api_compatibility/broker_api_v2.15_spec.rb

@@ -59,7 +59,7 @@
         end
 
         context 'when deprovisioning a service instance' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -81,7 +81,7 @@
         end
 
         context 'when updating a service instance' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -105,7 +105,7 @@
 
       describe 'service bindings' do
         context 'when creating a service binding' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -128,7 +128,7 @@
         end
 
         context 'when removing a service binding' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -182,7 +182,7 @@
         end
 
         context 'when updating a service instance' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -201,7 +201,7 @@
         end
 
         context 'when deleting a service instance' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -222,7 +222,7 @@
 
       describe 'service bindings' do
         context 'when creating a service binding' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -240,7 +240,7 @@
         end
 
         context 'when removing a service binding' do
-          let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
+          let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
 
           before do
             @service_instance_guid = service_instance.guid
@@ -403,7 +403,7 @@
       end
 
       context 'for bind route service' do
-        let(:route) { VCAP::CloudController::Route.make(space: @space) }
+        let(:route) { create(:route, space: @space) }
 
         before do
           create_route_binding(route)
@@ -514,11 +514,9 @@
 
       context 'when updating the service with the provided maintenance_info' do
         let(:service_instance) do
-          VCAP::CloudController::ManagedServiceInstance.make(
-            space_guid: @space_guid,
-            service_plan_guid: @plan_guid,
-            maintenance_info: { 'version' => '1.0.0' }
-          )
+          create(:managed_service_instance, space_guid: @space_guid,
+                                            service_plan_guid: @plan_guid,
+                                            maintenance_info: { 'version' => '1.0.0' })
         end
 
         before do