From 001aaf23f07cdae751d90785dcbd8b8440e4ed09 Mon Sep 17 00:00:00 2001 From: Philipp Thun Date: Mon, 4 May 2026 15:33:47 +0200 Subject: [PATCH] Fix two flaky test failures caused by test isolation issues BeforeEnqueueHook: Guard against duplicate plugin registration when the file is loaded multiple times. Duplicate callbacks caused multiple PollableJobModel records per enqueue instead of one. ManifestRouteUpdate: Use .present? instead of truthiness check for manifest_route[:options]. ManifestRoute.parse always sets options to {}, which is truthy but semantically empty. This caused spurious RouteUpdate calls that mutated route columns, breaking Sequel object equality in subsequent lookups. --- app/actions/manifest_route_update.rb | 2 +- lib/delayed_job_plugins/before_enqueue_hook.rb | 2 +- spec/unit/actions/manifest_route_update_spec.rb | 7 +++++++ .../before_enqueue_hook_spec.rb | 16 ++++++++++++++++ 4 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 spec/unit/lib/delayed_job_plugins/before_enqueue_hook_spec.rb diff --git a/app/actions/manifest_route_update.rb b/app/actions/manifest_route_update.rb index f506f3d3398..0bc4c1ca34d 100644 --- a/app/actions/manifest_route_update.rb +++ b/app/actions/manifest_route_update.rb @@ -102,7 +102,7 @@ def find_or_create_valid_route(app, manifest_route, user_audit_info) ) elsif !route.available_in_space?(app.space) raise InvalidRoute.new('Routes cannot be mapped to destinations in different spaces') - elsif manifest_route[:options] + elsif manifest_route[:options].present? message = RouteUpdateMessage.new({ 'options' => manifest_route[:options] }) diff --git a/lib/delayed_job_plugins/before_enqueue_hook.rb b/lib/delayed_job_plugins/before_enqueue_hook.rb index b295f11b103..cd58486cd91 100644 --- a/lib/delayed_job_plugins/before_enqueue_hook.rb +++ b/lib/delayed_job_plugins/before_enqueue_hook.rb @@ -5,4 +5,4 @@ class BeforeEnqueueHook < Delayed::Plugin end end end -Delayed::Worker.plugins << BeforeEnqueueHook +Delayed::Worker.plugins << BeforeEnqueueHook unless Delayed::Worker.plugins.include?(BeforeEnqueueHook) diff --git a/spec/unit/actions/manifest_route_update_spec.rb b/spec/unit/actions/manifest_route_update_spec.rb index a71604e6bea..cb08b5b78ab 100644 --- a/spec/unit/actions/manifest_route_update_spec.rb +++ b/spec/unit/actions/manifest_route_update_spec.rb @@ -49,6 +49,13 @@ module VCAP::CloudController end end + context 'when no options are provided' do + it 'does not call RouteUpdate' do + expect(RouteUpdate).not_to receive(:new) + ManifestRouteUpdate.update(app.guid, message, user_audit_info) + end + end + context 'when the new route has a protocol' do let(:message) do ManifestRoutesUpdateMessage.new( diff --git a/spec/unit/lib/delayed_job_plugins/before_enqueue_hook_spec.rb b/spec/unit/lib/delayed_job_plugins/before_enqueue_hook_spec.rb new file mode 100644 index 00000000000..df8174e2dce --- /dev/null +++ b/spec/unit/lib/delayed_job_plugins/before_enqueue_hook_spec.rb @@ -0,0 +1,16 @@ +require 'delayed_job' +require_relative '../../../../lib/delayed_job_plugins/before_enqueue_hook' + +RSpec.describe BeforeEnqueueHook do + it 'is registered in Delayed::Worker.plugins' do + expect(Delayed::Worker.plugins).to include(BeforeEnqueueHook) + end + + it 'does not register duplicate entries when loaded multiple times' do + count_before = Delayed::Worker.plugins.count(BeforeEnqueueHook) + load File.expand_path('../../../../lib/delayed_job_plugins/before_enqueue_hook.rb', __dir__) + count_after = Delayed::Worker.plugins.count(BeforeEnqueueHook) + + expect(count_after).to eq(count_before) + end +end