Skip to content

Commit a98ad52

Browse files
committed
Replace machinist with FactoryBot for test data setup
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.
1 parent 7fadd04 commit a98ad52

939 files changed

Lines changed: 14870 additions & 15459 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

Gemfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ end
6363

6464
group :test do
6565
gem 'codeclimate-test-reporter', '>= 1.0.8', require: false
66-
gem 'machinist', '~> 1.0.6'
66+
gem 'factory_bot', '~> 6.5'
6767
gem 'mock_redis'
6868
gem 'parallel_tests'
6969
gem 'rack-test'

Gemfile.lock

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ GEM
139139
erubi (1.13.1)
140140
excon (1.4.2)
141141
logger
142+
factory_bot (6.6.0)
143+
activesupport (>= 6.1.0)
142144
faraday (0.17.6)
143145
multipart-post (>= 1.2, < 3)
144146
faraday-cookie_jar (0.0.8)
@@ -286,7 +288,6 @@ GEM
286288
loofah (2.25.1)
287289
crass (~> 1.0.2)
288290
nokogiri (>= 1.12.0)
289-
machinist (1.0.6)
290291
method_source (1.1.0)
291292
mime-types (3.7.0)
292293
logger
@@ -622,6 +623,7 @@ DEPENDENCIES
622623
concurrent-ruby
623624
debug (~> 1.11)
624625
digest-xxhash
626+
factory_bot (~> 6.5)
625627
fluent-logger
626628
fog-aliyun
627629
fog-aws
@@ -636,7 +638,6 @@ DEPENDENCIES
636638
json-diff
637639
json-schema
638640
listen
639-
machinist (~> 1.0.6)
640641
mime-types (~> 3.7)
641642
mock_redis
642643
multipart-parser

lib/cloud_controller/console.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@
3636

3737
if ENV['NEW_RELIC_ENV'] == 'development'
3838
$LOAD_PATH.unshift(File.expand_path('../../spec/support', __dir__))
39-
require 'machinist/sequel'
40-
require 'machinist/object'
41-
require 'fakes/blueprints'
39+
require 'factories'
40+
require 'sham_shim'
4241
end
4342

4443
module VCAP::CloudController

spec/acceptance/async_bindings_spec.rb

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -33,16 +33,16 @@ module VCAP::CloudController
3333
end
3434

3535
context 'when a service instance is shared' do
36-
let(:service_instance) { ManagedServiceInstance.make }
37-
let(:target_space) { Space.make }
36+
let(:service_instance) { create(:managed_service_instance) }
37+
let(:target_space) { create(:space) }
3838

3939
before do
4040
service_instance.add_shared_space(target_space)
4141
end
4242

4343
context 'when there are bindings in the target space' do
44-
let(:target_app) { AppModel.make(space: target_space) }
45-
let!(:target_binding) { ServiceBinding.make(app: target_app, service_instance: service_instance) }
44+
let(:target_app) { create(:app_model, space: target_space) }
45+
let!(:target_binding) { create(:service_binding, app: target_app, service_instance: service_instance) }
4646

4747
it 'issues an unbind and fails the instance deletion if the service instance is deleted recursively and accepts_incomplete is true' do
4848
delete("/v2/service_instances/#{service_instance.guid}", 'recursive=true&accepts_incomplete=true', admin_headers)
@@ -70,8 +70,8 @@ module VCAP::CloudController
7070

7171
context 'and when there are bindings in the source space' do
7272
let(:source_space) { service_instance.space }
73-
let(:source_app) { AppModel.make(space: source_space) }
74-
let!(:source_binding) { ServiceBinding.make(app: source_app, service_instance: service_instance) }
73+
let(:source_app) { create(:app_model, space: source_space) }
74+
let!(:source_binding) { create(:service_binding, app: source_app, service_instance: service_instance) }
7575

7676
it 'issues unbinds and fails the instance deletion if the service instance is deleted recursively and accepts_incomplete is true' do
7777
delete("/v2/service_instances/#{service_instance.guid}", 'recursive=true&accepts_incomplete=true', admin_headers)
@@ -121,15 +121,15 @@ module VCAP::CloudController
121121

122122
context 'when DELETE /v3/apps/:guid is called' do
123123
context 'and multiple service bindings exist' do
124-
let(:space) { Space.make }
125-
let(:app_model) { VCAP::CloudController::AppModel.make(name: 'app_name', space: space) }
126-
let(:package) { VCAP::CloudController::PackageModel.make(app: app_model) }
127-
let!(:droplet) { VCAP::CloudController::DropletModel.make(package: package, app: app_model) }
128-
let!(:process) { VCAP::CloudController::ProcessModel.make(app: app_model) }
129-
let!(:deployment) { VCAP::CloudController::DeploymentModel.make(app: app_model) }
124+
let(:space) { create(:space) }
125+
let(:app_model) { create(:app_model, name: 'app_name', space: space) }
126+
let(:package) { create(:package_model, app: app_model) }
127+
let!(:droplet) { create(:droplet_model, package: package, app: app_model) }
128+
let!(:process) { create(:process_model, app: app_model) }
129+
let!(:deployment) { create(:deployment_model, app: app_model) }
130130

131-
let!(:service_binding1) { ServiceBinding.make(app: app_model, service_instance: ManagedServiceInstance.make(space:)) }
132-
let!(:service_binding2) { ServiceBinding.make(app: app_model, service_instance: ManagedServiceInstance.make(space:)) }
131+
let!(:service_binding1) { create(:service_binding, app: app_model, service_instance: create(:managed_service_instance, space:)) }
132+
let!(:service_binding2) { create(:service_binding, app: app_model, service_instance: create(:managed_service_instance, space:)) }
133133

134134
it 'returns a list of errors for the service bindings' do
135135
delete("/v3/apps/#{app_model.guid}", nil, admin_headers)
@@ -157,8 +157,8 @@ module VCAP::CloudController
157157
context 'and multiple service bindings exist' do
158158
let(:process) { ProcessModelFactory.make }
159159

160-
let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
161-
let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
160+
let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
161+
let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
162162

163163
it 'returns a concatenated error for the service bindings' do
164164
delete("/v2/apps/#{process.app.guid}", 'recursive=true', admin_headers)
@@ -180,8 +180,8 @@ module VCAP::CloudController
180180
context 'and multiple service bindings exist' do
181181
let(:process) { ProcessModelFactory.make }
182182

183-
let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
184-
let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
183+
let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
184+
let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
185185

186186
it 'returns a concatenated error for the service bindings' do
187187
delete("/v2/spaces/#{process.space.guid}", 'recursive=true', admin_headers)
@@ -203,8 +203,8 @@ module VCAP::CloudController
203203
context 'and multiple service bindings exist' do
204204
let(:process) { ProcessModelFactory.make }
205205

206-
let!(:service_binding1) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
207-
let!(:service_binding2) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
206+
let!(:service_binding1) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
207+
let!(:service_binding2) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
208208

209209
it 'returns a concatenated error for the service bindings' do
210210
delete("/v2/organizations/#{process.organization.guid}", 'recursive=true', admin_headers)
@@ -224,8 +224,8 @@ module VCAP::CloudController
224224

225225
context 'when PUT /v2/service_instances/:guid is called and when an async binding operation is in progress' do
226226
let(:process) { ProcessModelFactory.make }
227-
let(:service_binding) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
228-
let!(:service_binding_operation) { ServiceBindingOperation.make(state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
227+
let(:service_binding) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
228+
let!(:service_binding_operation) { create(:service_binding_operation, state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
229229

230230
let(:body) do
231231
{
@@ -270,8 +270,8 @@ module VCAP::CloudController
270270

271271
context 'when DELETE /v2/service_instances/:guid is called and when an async binding operation is in progress' do
272272
let(:process) { ProcessModelFactory.make }
273-
let(:service_binding) { ServiceBinding.make(app: process.app, service_instance: ManagedServiceInstance.make(space: process.space)) }
274-
let!(:service_binding_operation) { ServiceBindingOperation.make(state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
273+
let(:service_binding) { create(:service_binding, app: process.app, service_instance: create(:managed_service_instance, space: process.space)) }
274+
let!(:service_binding_operation) { create(:service_binding_operation, state: 'in progress', type: operation_type, service_binding_id: service_binding.id) }
275275

276276
context 'when the binding operation is create' do
277277
let(:operation_type) { 'create' }

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@
227227
let(:catalog) { default_catalog(plan_updateable: true) }
228228

229229
context 'service broker registration' do
230-
let(:user) { VCAP::CloudController::User.make }
230+
let(:user) { create(:user) }
231231

232232
before do
233233
setup_broker_with_user(user)
@@ -246,7 +246,7 @@
246246
end
247247

248248
context 'service provision request' do
249-
let(:user) { VCAP::CloudController::User.make }
249+
let(:user) { create(:user) }
250250

251251
before do
252252
provision_service(user:)
@@ -264,7 +264,7 @@
264264
end
265265

266266
context 'service deprovision request' do
267-
let(:user) { VCAP::CloudController::User.make }
267+
let(:user) { create(:user) }
268268

269269
before do
270270
provision_service(user:)
@@ -283,7 +283,7 @@
283283
end
284284

285285
context 'service update request' do
286-
let(:user) { VCAP::CloudController::User.make }
286+
let(:user) { create(:user) }
287287

288288
before do
289289
provision_service(user:)
@@ -302,7 +302,7 @@
302302
end
303303

304304
context 'service binding request' do
305-
let(:user) { VCAP::CloudController::User.make }
305+
let(:user) { create(:user) }
306306

307307
before do
308308
provision_service
@@ -322,7 +322,7 @@
322322
end
323323

324324
context 'service unbind request' do
325-
let(:user) { VCAP::CloudController::User.make }
325+
let(:user) { create(:user) }
326326
let(:async) { false }
327327

328328
before do
@@ -367,7 +367,7 @@
367367
end
368368

369369
context 'create service key request' do
370-
let(:user) { VCAP::CloudController::User.make }
370+
let(:user) { create(:user) }
371371

372372
before do
373373
provision_service
@@ -386,7 +386,7 @@
386386
end
387387

388388
context 'delete service key request' do
389-
let(:user) { VCAP::CloudController::User.make }
389+
let(:user) { create(:user) }
390390

391391
before do
392392
provision_service
@@ -407,8 +407,8 @@
407407

408408
context 'create route binding' do
409409
let(:catalog) { default_catalog(plan_updateable: true, requires: ['route_forwarding']) }
410-
let(:user) { VCAP::CloudController::User.make }
411-
let(:route) { VCAP::CloudController::Route.make(space: @space) }
410+
let(:user) { create(:user) }
411+
let(:route) { create(:route, space: @space) }
412412

413413
before do
414414
provision_service
@@ -428,8 +428,8 @@
428428

429429
context 'delete route binding' do
430430
let(:catalog) { default_catalog(plan_updateable: true, requires: ['route_forwarding']) }
431-
let(:user) { VCAP::CloudController::User.make }
432-
let(:route) { VCAP::CloudController::Route.make(space: @space) }
431+
let(:user) { create(:user) }
432+
let(:route) { create(:route, space: @space) }
433433

434434
before do
435435
provision_service
@@ -449,9 +449,9 @@
449449
end
450450

451451
context 'when multiple users operate on a service instance' do
452-
let(:user_a) { VCAP::CloudController::User.make }
453-
let(:user_b) { VCAP::CloudController::User.make }
454-
let(:user_c) { VCAP::CloudController::User.make }
452+
let(:user_a) { create(:user) }
453+
let(:user_b) { create(:user) }
454+
let(:user_c) { create(:user) }
455455

456456
before do
457457
provision_service(user: user_a)
@@ -547,7 +547,7 @@
547547

548548
context 'for bind route service' do
549549
let(:catalog) { default_catalog(requires: ['route_forwarding']) }
550-
let(:route) { VCAP::CloudController::Route.make(space: @space) }
550+
let(:route) { create(:route, space: @space) }
551551

552552
before do
553553
provision_service

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
end
4949

5050
it 'sends the broker the X-Broker-Api-Originating-Identity header' do
51-
user = VCAP::CloudController::User.make
51+
user = create(:user)
5252

5353
get("/v2/service_bindings/#{@binding_guid}/parameters",
5454
{}.to_json,
@@ -249,7 +249,7 @@
249249
end
250250

251251
describe 'update service dashboard url' do
252-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
252+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
253253
let(:catalog) { default_catalog(plan_updateable: true) }
254254

255255
before do

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

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
end
6060

6161
context 'when deprovisioning a service instance' do
62-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
62+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
6363

6464
before do
6565
@service_instance_guid = service_instance.guid
@@ -81,7 +81,7 @@
8181
end
8282

8383
context 'when updating a service instance' do
84-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
84+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
8585

8686
before do
8787
@service_instance_guid = service_instance.guid
@@ -105,7 +105,7 @@
105105

106106
describe 'service bindings' do
107107
context 'when creating a service binding' do
108-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
108+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
109109

110110
before do
111111
@service_instance_guid = service_instance.guid
@@ -128,7 +128,7 @@
128128
end
129129

130130
context 'when removing a service binding' do
131-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
131+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
132132

133133
before do
134134
@service_instance_guid = service_instance.guid
@@ -182,7 +182,7 @@
182182
end
183183

184184
context 'when updating a service instance' do
185-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
185+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
186186

187187
before do
188188
@service_instance_guid = service_instance.guid
@@ -201,7 +201,7 @@
201201
end
202202

203203
context 'when deleting a service instance' do
204-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
204+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
205205

206206
before do
207207
@service_instance_guid = service_instance.guid
@@ -222,7 +222,7 @@
222222

223223
describe 'service bindings' do
224224
context 'when creating a service binding' do
225-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
225+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
226226

227227
before do
228228
@service_instance_guid = service_instance.guid
@@ -240,7 +240,7 @@
240240
end
241241

242242
context 'when removing a service binding' do
243-
let(:service_instance) { VCAP::CloudController::ManagedServiceInstance.make(space_guid: @space_guid, service_plan_guid: @plan_guid) }
243+
let(:service_instance) { create(:managed_service_instance, space_guid: @space_guid, service_plan_guid: @plan_guid) }
244244

245245
before do
246246
@service_instance_guid = service_instance.guid
@@ -403,7 +403,7 @@
403403
end
404404

405405
context 'for bind route service' do
406-
let(:route) { VCAP::CloudController::Route.make(space: @space) }
406+
let(:route) { create(:route, space: @space) }
407407

408408
before do
409409
create_route_binding(route)
@@ -514,11 +514,9 @@
514514

515515
context 'when updating the service with the provided maintenance_info' do
516516
let(:service_instance) do
517-
VCAP::CloudController::ManagedServiceInstance.make(
518-
space_guid: @space_guid,
519-
service_plan_guid: @plan_guid,
520-
maintenance_info: { 'version' => '1.0.0' }
521-
)
517+
create(:managed_service_instance, space_guid: @space_guid,
518+
service_plan_guid: @plan_guid,
519+
maintenance_info: { 'version' => '1.0.0' })
522520
end
523521

524522
before do

0 commit comments

Comments
 (0)