Factory bot migration

Gemfile

@@ -63,7 +63,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

@@ -139,6 +139,8 @@ GEM
     erubi (1.13.1)
     excon (1.4.2)
       logger
+    factory_bot (6.6.0)
+      activesupport (>= 6.1.0)
     faraday (0.17.6)
       multipart-post (>= 1.2, < 3)
     faraday-cookie_jar (0.0.8)
@@ -286,7 +288,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
@@ -622,6 +623,7 @@ DEPENDENCIES
   concurrent-ruby
   debug (~> 1.11)
   digest-xxhash
+  factory_bot (~> 6.5)
   fluent-logger
   fog-aliyun
   fog-aws
@@ -636,7 +638,6 @@ DEPENDENCIES
   json-diff
   json-schema
   listen
-  machinist (~> 1.0.6)
   mime-types (~> 3.7)
   mock_redis
   multipart-parser

decisions/0002-using-machinist-for-factories.md

@@ -6,7 +6,7 @@ Date: 2019-03-22
 Status
 ------
 
-Accepted
+Superseded by [ADR 0015](0015-using-factory-bot-for-factories.md)
 
 
 Context

decisions/0015-using-factory-bot-for-factories.md

@@ -0,0 +1,107 @@
+# 15: Using FactoryBot for Factories
+
+Date: 2026-05-23
+
+## Status
+
+Accepted (supersedes [ADR 0002](0002-using-machinist-for-factories.md))
+
+## Context
+
+[ADR 0002](0002-using-machinist-for-factories.md) (2019) decided to keep
+[machinist][machinist] after a partial migration to [factory_bot][factory_bot]
+hit friction with Sequel's mutual-foreign-key associations. Seven years later,
+the situation has shifted:
+
+* `machinist` 1.0.6 has had no upstream activity since 2013. The "maintained
+  fork" mentioned in 0002 never materialised in a way that this project
+  consumed.
+* `machinist` 2.0 exists but is not a viable upgrade. It removed the Sequel
+  adapter we depend on, dropped `Sham`, and flipped `make` from a persisting
+  call to a non-persisting one — every existing call site would change
+  meaning.
+* `factory_bot` is actively maintained and well-known to anyone joining a
+  Ruby project.
+* The blocker described in 0002 — Sequel's mutual-foreign-key pattern, where
+  two records reference each other — turns out to be solvable cleanly in
+  `factory_bot` once a global `to_create` and `after(:create)` callbacks
+  with transient flags are used. None of these primitives required a special
+  Sequel adapter.
+
+## Decision
+
+Replace `machinist` with `factory_bot` as the test-data framework. The
+`machinist` gem and its supporting files (`spec/support/fakes/blueprints.rb`,
+`spec/support/machinist_monkey_patch.rb`) are removed; ~11k call sites of
+`Klass.make(...)` and `Klass.make_unsaved(...)` are converted to
+`create(:klass, ...)` and `build(:klass, ...)`; and ~130 blueprints become
+factory definitions under `spec/support/factory_definitions/`.
+
+The conversion is done as one change. There is no extended period in which
+both libraries coexist in the codebase.
+
+### Key technical decisions
+
+The patterns below are what made the 2019 friction tractable.
+
+* **Global `to_create { |i| i.save; i.refresh }`** in
+  `spec/support/factories.rb`. This matches machinist's Sequel adapter,
+  which both saved and refreshed. Without the refresh, tests that mutate
+  associations after creation see Sequel's stale in-memory association
+  cache rather than the current DB state.
+
+* **`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. Call sites move from
+  `Foo.make(:bar, x: 1)` to `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.
+
+## Consequences
+
+* New contributors no longer need to learn `machinist` first; `factory_bot`
+  is the de-facto Ruby standard.
+* The `machinist 1.0.6` dependency and its dependabot churn are gone.
+* `factory_bot` is actively maintained, so most future test-framework
+  upgrades happen via `bundle update` rather than via a custom monkey
+  patch (as `machinist_monkey_patch.rb` had to do).
+* Tooling that reasoned about `machinist` blueprints (e.g. spec generators,
+  custom rubocop cops) needs to be updated; none of it lived in this
+  repository.
+
+## Alternatives Considered
+
+* **Stay on `machinist 1.0.6`.** Rejected: unmaintained upstream, dependabot
+  noise, and the risk that some future Ruby/Sequel upgrade silently breaks
+  the gem.
+* **Upgrade to `machinist 2.0`.** Rejected: no Sequel adapter, no `Sham`,
+  and `make` no longer persists — the upgrade is effectively a rewrite of
+  every call site for less benefit than moving to `factory_bot`.
+* **Adopt a maintained fork of `machinist`.** No fork with meaningful
+  activity exists, and adopting one trades one unmaintained dependency for
+  another small one.
+
+[machinist]: https://github.com/notahat/machinist
+[factory_bot]: https://github.com/thoughtbot/factory_bot

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