Skip to content

Commit 2e21a18

Browse files
bug fixes
1 parent d9df392 commit 2e21a18

6 files changed

Lines changed: 288 additions & 12 deletions

File tree

‎ruby/lib/ci/queue/class_proxy.rb‎

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@ module Queue
66
# This allows Class objects to be sent over DRb in lazy loading scenarios where the
77
# actual Class object can't be marshaled.
88
class ClassProxy < BasicObject
9-
def initialize(klass_or_name)
9+
# Track loaded files across all ClassProxy instances to prevent duplicate loads
10+
@@loaded_files = ::Set.new
11+
@@load_mutex = ::Mutex.new
12+
13+
def initialize(klass_or_name, file_path: nil)
1014
@class_name = klass_or_name.is_a?(::String) ? klass_or_name : klass_or_name.to_s
15+
@file_path = file_path
1116
@klass = nil
1217
end
1318

@@ -49,20 +54,67 @@ def inspect
4954
target_class.inspect
5055
end
5156

52-
# Custom marshaling: serialize as class name string
57+
# Custom marshaling: serialize as class name string and file path
5358
def marshal_dump
54-
@class_name
59+
{ class_name: @class_name, file_path: @file_path }
5560
end
5661

57-
def marshal_load(class_name)
58-
@class_name = class_name
62+
def marshal_load(data)
63+
@class_name = data[:class_name]
64+
@file_path = data[:file_path]
5965
@klass = nil
6066
end
6167

6268
private
6369

6470
def target_class
65-
@klass ||= @class_name.split('::').reduce(::Object) { |mod, const| mod.const_get(const) }
71+
return @klass if @klass
72+
73+
# Try to resolve the constant
74+
@klass = resolve_constant(@class_name)
75+
rescue ::NameError => e
76+
# If constant not found and we have a file path, try loading the file
77+
if @file_path
78+
load_test_file(@file_path)
79+
# Retry constant lookup after loading
80+
begin
81+
@klass = resolve_constant(@class_name)
82+
rescue ::NameError => retry_error
83+
# File loaded but class still not found - provide helpful error
84+
::Kernel.raise ::NameError,
85+
"Class #{@class_name} not found after loading #{@file_path}. " \
86+
"The file may not define the expected class. Original error: #{retry_error.message}"
87+
end
88+
else
89+
::Kernel.raise e
90+
end
91+
end
92+
93+
def resolve_constant(class_name)
94+
class_name.split('::').reduce(::Object) { |mod, const| mod.const_get(const) }
95+
end
96+
97+
def load_test_file(file_path)
98+
# Validate file path for security
99+
unless file_path.end_with?('.rb')
100+
::Kernel.raise ::ArgumentError, "Invalid test file path (must end with .rb): #{file_path}"
101+
end
102+
103+
# Check if file exists
104+
unless ::File.exist?(file_path)
105+
::Kernel.raise ::LoadError, "Test file not found: #{file_path}"
106+
end
107+
108+
# Thread-safe file loading with deduplication
109+
return if @@loaded_files.include?(file_path)
110+
111+
@@load_mutex.synchronize do
112+
# Double-check inside mutex to prevent race condition
113+
return if @@loaded_files.include?(file_path)
114+
115+
::Kernel.load(file_path)
116+
@@loaded_files.add(file_path)
117+
end
66118
end
67119
end
68120
end

‎ruby/lib/ci/queue/redis/worker.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,9 +191,9 @@ def load_test_from_entry(test_id)
191191
@lazy_loader.load_class(class_name)
192192
end
193193

194-
# Create and return the SingleExample
194+
# Create and return the SingleExample with file path for lazy loading in workers
195195
runnable = @lazy_loader.find_class(class_name)
196-
Minitest::Queue::SingleExample.new(runnable, method_name)
196+
Minitest::Queue::SingleExample.new(runnable, method_name, file_path: file_path)
197197
end
198198

199199
def fetch_manifest_for_lazy_load

‎ruby/lib/minitest/queue.rb‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -222,17 +222,18 @@ def rescue_run_errors(&block)
222222
end
223223

224224
class SingleExample
225-
attr_reader :method_name
225+
attr_reader :method_name, :file_path
226226

227-
def initialize(runnable, method_name)
227+
def initialize(runnable, method_name, file_path: nil)
228228
# Store class name as string for DRb serialization
229229
@runnable_name = runnable.is_a?(String) ? runnable : runnable.to_s
230230
@method_name = method_name
231+
@file_path = file_path
231232
end
232233

233234
# Returns a ClassProxy that behaves like a Class but marshals safely over DRb
234235
def runnable
235-
@runnable ||= CI::Queue::ClassProxy.new(@runnable_name)
236+
@runnable ||= CI::Queue::ClassProxy.new(@runnable_name, file_path: @file_path)
236237
end
237238

238239
# Provides access to class name as string (for DRb serialization)
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
# frozen_string_literal: true
2+
require 'test_helper'
3+
require 'tempfile'
4+
5+
module CI
6+
module Queue
7+
class ClassProxyTest < Minitest::Test
8+
def setup
9+
# Clear loaded files tracking between tests
10+
ClassProxy.class_variable_set(:@@loaded_files, Set.new)
11+
end
12+
13+
def test_acts_like_a_class
14+
proxy = ClassProxy.new('String')
15+
16+
assert proxy.is_a?(Class)
17+
assert proxy == String, "ClassProxy should equal the actual class"
18+
assert_equal 'String', proxy.name
19+
end
20+
21+
def test_delegates_method_calls
22+
proxy = ClassProxy.new('Array')
23+
24+
assert_respond_to proxy, :new
25+
instance = proxy.new
26+
assert_kind_of Array, instance
27+
end
28+
29+
def test_marshaling_without_file_path
30+
proxy = ClassProxy.new('Integer')
31+
32+
marshaled = Marshal.dump(proxy)
33+
unmarshaled = Marshal.load(marshaled)
34+
35+
assert_equal 'Integer', unmarshaled.name
36+
assert unmarshaled == Integer, "Unmarshaled proxy should equal Integer"
37+
end
38+
39+
def test_marshaling_with_file_path
40+
proxy = ClassProxy.new('String', file_path: '/tmp/test.rb')
41+
42+
marshaled = Marshal.dump(proxy)
43+
unmarshaled = Marshal.load(marshaled)
44+
45+
assert_equal 'String', unmarshaled.name
46+
end
47+
48+
def test_lazy_loads_file_when_constant_not_found
49+
test_file = create_test_file('LazyLoadTest', 'class LazyLoadTest; end')
50+
51+
# Create proxy before class is loaded
52+
proxy = ClassProxy.new('LazyLoadTest', file_path: test_file.path)
53+
54+
# Accessing proxy should trigger file load
55+
assert_equal 'LazyLoadTest', proxy.name
56+
57+
cleanup_test_file(test_file)
58+
end
59+
60+
def test_loads_file_only_once
61+
load_counter = 0
62+
test_file = create_test_file('LoadOnceTest', <<~RUBY)
63+
class LoadOnceTest
64+
@@load_count ||= 0
65+
@@load_count += 1
66+
def self.load_count
67+
@@load_count
68+
end
69+
end
70+
RUBY
71+
72+
# Create two proxies for same class
73+
proxy1 = ClassProxy.new('LoadOnceTest', file_path: test_file.path)
74+
proxy2 = ClassProxy.new('LoadOnceTest', file_path: test_file.path)
75+
76+
# Access both
77+
count1 = proxy1.load_count
78+
count2 = proxy2.load_count
79+
80+
# File should only be loaded once
81+
assert_equal 1, count2
82+
83+
cleanup_test_file(test_file)
84+
end
85+
86+
def test_raises_load_error_for_missing_file
87+
proxy = ClassProxy.new('MissingClass', file_path: '/tmp/nonexistent_file_12345.rb')
88+
89+
error = assert_raises(LoadError) do
90+
proxy.name
91+
end
92+
93+
assert_match(/Test file not found/, error.message)
94+
assert_match(/nonexistent_file_12345\.rb/, error.message)
95+
end
96+
97+
def test_raises_argument_error_for_invalid_file_extension
98+
proxy = ClassProxy.new('InvalidFile', file_path: '/tmp/test.txt')
99+
100+
error = assert_raises(ArgumentError) do
101+
proxy.name
102+
end
103+
104+
assert_match(/Invalid test file path/, error.message)
105+
assert_match(/must end with \.rb/, error.message)
106+
end
107+
108+
def test_raises_helpful_error_when_class_not_defined_in_file
109+
test_file = create_test_file('WrongClass', 'class WrongClass; end')
110+
111+
proxy = ClassProxy.new('ExpectedClass', file_path: test_file.path)
112+
113+
error = assert_raises(NameError) do
114+
proxy.name
115+
end
116+
117+
assert_match(/ExpectedClass not found after loading/, error.message)
118+
assert_match(/#{test_file.path}/, error.message)
119+
120+
cleanup_test_file(test_file)
121+
end
122+
123+
def test_thread_safety
124+
test_file = create_test_file('ThreadSafeClass', 'class ThreadSafeClass; end')
125+
126+
threads = 10.times.map do
127+
Thread.new do
128+
proxy = ClassProxy.new('ThreadSafeClass', file_path: test_file.path)
129+
proxy.name
130+
end
131+
end
132+
133+
results = threads.map(&:value)
134+
135+
assert_equal 10, results.size
136+
assert results.all? { |r| r == 'ThreadSafeClass' }
137+
138+
cleanup_test_file(test_file)
139+
end
140+
141+
def test_equality_with_actual_class
142+
proxy = ClassProxy.new('String')
143+
144+
# ClassProxy#== handles comparison with actual class
145+
assert proxy == String, "ClassProxy should equal String"
146+
147+
# Note: String#== doesn't know about ClassProxy, so String == proxy may not work
148+
# This is expected behavior - the proxy is transparent when used polymorphically
149+
end
150+
151+
def test_equality_with_another_proxy
152+
proxy1 = ClassProxy.new('String')
153+
proxy2 = ClassProxy.new('String')
154+
155+
# Both proxies should equal the same underlying class
156+
assert proxy1 == String, "proxy1 should equal String"
157+
assert proxy2 == String, "proxy2 should equal String"
158+
end
159+
160+
def test_hash_delegation
161+
proxy = ClassProxy.new('Integer')
162+
163+
assert_equal Integer.hash, proxy.hash
164+
end
165+
166+
def test_inspect_delegation
167+
proxy = ClassProxy.new('Array')
168+
169+
assert_equal Array.inspect, proxy.inspect
170+
end
171+
172+
def test_to_s_returns_class_name
173+
proxy = ClassProxy.new('String')
174+
175+
assert_equal 'String', proxy.to_s
176+
end
177+
178+
def test_nested_constant_resolution
179+
proxy = ClassProxy.new('Minitest::Test')
180+
181+
assert proxy == Minitest::Test, "ClassProxy should equal Minitest::Test"
182+
assert_equal 'Minitest::Test', proxy.name
183+
end
184+
185+
def test_lazy_loads_nested_constant
186+
test_file = create_test_file('Nested::LazyClass', <<~RUBY)
187+
module Nested
188+
class LazyClass
189+
def self.test_method
190+
'it works'
191+
end
192+
end
193+
end
194+
RUBY
195+
196+
proxy = ClassProxy.new('Nested::LazyClass', file_path: test_file.path)
197+
198+
assert_equal 'it works', proxy.test_method
199+
200+
cleanup_test_file(test_file)
201+
end
202+
203+
private
204+
205+
def create_test_file(class_name, content)
206+
file = Tempfile.new(['test_class', '.rb'])
207+
file.write(content)
208+
file.close
209+
file
210+
end
211+
212+
def cleanup_test_file(file)
213+
# Remove the constant if it was defined
214+
parts = file.path.match(/test_class.*\.rb/)[0].split('::')
215+
Object.send(:remove_const, parts.first.to_sym) if Object.const_defined?(parts.first)
216+
rescue
217+
# Ignore cleanup errors
218+
ensure
219+
file.unlink
220+
end
221+
end
222+
end
223+
end

‎ruby/test/fixtures/log/bisect_test_details.log‎

Whitespace-only changes.

‎ruby/test/minitest/queue/single_example_test.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ def test_drb_serialization_simulated
7979
begin
8080
# Connect as client
8181
DRb.start_service
82-
remote_queue = DRbObject.new_from_uri('druby://localhost:9999')
82+
remote_queue = DRbObject.new(nil, 'druby://localhost:9999')
8383

8484
# Create example and try to send it over DRb
8585
example = SingleExample.new(@example_class, 'test_example')

0 commit comments

Comments
 (0)