Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rrrspec-client/lib/rrrspec/client.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'rrrspec'
require 'rrrspec/registry'
require 'rrrspec/client/configuration'
require 'rrrspec/client/rspec_runner'
require 'rrrspec/client/slave_runner'
Expand Down
13 changes: 13 additions & 0 deletions rrrspec-client/lib/rrrspec/client/rspec_runner_factory.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
require 'rrrspec/client/rspec_runner'

module RRRSpec
module Client
class RSpecRunnerFactory

def create
return RSpecRunner.new
end

end
end
end
16 changes: 12 additions & 4 deletions rrrspec-client/lib/rrrspec/client/slave_runner.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ def initialize(slave, working_dir, taskset_key)
@slave = slave
@taskset = Taskset.new(taskset_key)
@timeout = TASKQUEUE_TASK_TIMEOUT
@rspec_runner = RSpecRunner.new
@runners = Hash.new do |runners, file_ext|
runner_factory = RRRSpec::Registry.get_runner_factory(file_ext)
runners[file_ext] = runner_factory.create
end
@working_path = File.join(working_dir, @taskset.rsync_name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@runners = Hash.new { |h, k| h[k] = RRRSpec::Registry.create_runner(k) }

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you prefer to copy them to a object var?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted above, I don't want to put an RSpecRunner object in a global state. I want to make runner instances local to SlaveRunner.

In general, using global variables is not good design. It's better to make the scope narrow as much as possible. Plus, if you need to have some object in a wider scope, it's better to make the object owner clear in order to control the object lifecycle.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@unknown_spec_timeout_sec = @taskset.unknown_spec_timeout_sec
@least_timeout_sec = @taskset.least_timeout_sec
Expand Down Expand Up @@ -57,9 +60,12 @@ def work
@timeout = TASKQUEUE_TASK_TIMEOUT
trial = Trial.create(task, @slave)

@rspec_runner.reset
task_file_ext = File.extname(task.spec_file)
runner = @runners[task_file_ext]

runner.reset
$0 = "rrrspec slave[#{ENV['SLAVE_NUMBER']}]: setting up #{task.spec_file}"
status, outbuf, errbuf = @rspec_runner.setup(File.join(@working_path, task.spec_file))
status, outbuf, errbuf = runner.setup(File.join(@working_path, task.spec_file))
unless status
trial.finish('error', outbuf, errbuf, nil, nil, nil)
ArbiterQueue.trial(trial)
Expand All @@ -75,7 +81,7 @@ def work
hard_timeout_sec, TIMEOUT_EXITCODE
) do
Timeout::timeout(soft_timeout_sec, SoftTimeoutException) do
@rspec_runner.run(formatter)
runner.run(formatter)
end
end
if status
Expand Down Expand Up @@ -130,6 +136,8 @@ def example_pending

def example_failed(notification)
@failed += 1
return if !notification.respond_to?(:exception)

if notification.exception.is_a?(SoftTimeoutException)
@timeout = true
end
Expand Down
34 changes: 34 additions & 0 deletions rrrspec-client/lib/rrrspec/registry.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
require 'rrrspec/client/rspec_runner'
require 'rrrspec/client/rspec_runner_factory'

module RRRSpec
module Registry

class NoFactoryForFileExtensionException < Exception

def initialize(file_extension)
super "There is no factory for #{file_extension}"
end
end

@@runner_factories = Hash.new

def self.reset_runner_factories
@@runner_factories = []
end

def self.register_runner_factory(runner_factory, file_extension)
@@runner_factories[file_extension] = runner_factory
end


def self.get_runner_factory(file_extension)
raise NoFactoryForFileExtensionException, file_extension unless @@runner_factories[file_extension]

@@runner_factories[file_extension].new
end

end

Registry.register_runner_factory RRRSpec::Client::RSpecRunnerFactory, '.rb'
end
15 changes: 15 additions & 0 deletions rrrspec-client/spec/rrrspec/registry_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
module RRRSpec

RSpec.describe Registry do
subject { Registry }

it 'should return a runner on .rb' do
expect(subject.get_runner_factory('.rb')).to be_truthy
end

it 'should raise exception on .feature' do
expect { subject.get_runner_factory('.feature') }.to raise_error('There is no factory for .feature')
end
end

end