Skip to content

add support for registering runners#62

Open
jrluis wants to merge 1 commit intocookpad:masterfrom
jrluis:cucumber-support
Open

add support for registering runners#62
jrluis wants to merge 1 commit intocookpad:masterfrom
jrluis:cucumber-support

Conversation

@jrluis
Copy link

@jrluis jrluis commented Jul 29, 2016

This runs cucumber tests mixed with rspec tests.

To add the cucumber plugin add rrrspec-cucumber to your gemfile

gem 'rrrspec-cucumber'

Then in .rrrspec configure the runner chain to include rrrspec-cucumber

require 'rrrspec/cucumber'

RRRSpec::Registry.register_runner RRRSpec::Cucumber::CucumberRunnerFactory.new, '.feature'

You can checkout the plugin code at https://github.com/Seedrs/rrrspec-cucumber

For the slave to load the .rrrspec configuration the slave command also has to be changed

  conf.slave_command = <<-SLAVE
    bundle exec rrrspec-client slave --config .rrrspec
  SLAVE

Finally, add cucumber to a rails project and in .rrrspec also select the .features

conf.spec_files = lambda do
    Dir.chdir(conf.packaging_dir) do
        Dir['features/**{,/*/**}/*.feature'].uniq
    end
end


@rspec_runner.reset
task_file_ext = File.extname(task.spec_file)
runner = RunnerFactory.create(task_file_ext)
Copy link
Contributor

Choose a reason for hiding this comment

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

BUG: RSpecRunner has a before_suite_run flag for oneshot suite-wide initialization. New-ing the runner for each task resets this flag and causes multiple initialization.

Copy link
Author

Choose a reason for hiding this comment

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

I've made a chain of runner factories, to allow plugin the runners.
The RSpecRunnerFactory has the runner as a singleton, that way the bug is gone. I've also added a spec to avoid this bug in the future.

@jrluis jrluis force-pushed the cucumber-support branch from 486e67c to f879a1f Compare August 2, 2016 14:04
module RRRSpec
module Client
class RSpecRunnerFactory
attr_reader :next_factory
Copy link
Contributor

Choose a reason for hiding this comment

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

If the factory implementation can be switched by the file extension, isn't it better to have a registry type plugin structure instead of chaining?

Copy link
Author

Choose a reason for hiding this comment

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

A registry would be a nice solution, but it would limit how the choice of the runner is made.

Currently the choice is made in the plugin, if we change it to a registry, the choice would be made be rrrspec and not in the plugin.

My doubt is if we can find a general rule that works for all future cases so that the choice can be made in rrrspec code and not in the plugin.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's really hard to find a "general rule" based on a file extension. It's just a few characters. If there's no usage for that flexibility right now, it's better to keep it simple. Chaining is a good solution if the argument needs to be decorated (e.g. Java's Servlet API), but in this case it seems no use to me. If you should need more flexibility, it's better to let users to specify the factory directly and let them write in a composition style, instead of forcing chaining.

Copy link
Author

Choose a reason for hiding this comment

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

Added a registry for runner factories

@jrluis jrluis force-pushed the cucumber-support branch 2 times, most recently from 2f19e5f to d45f697 Compare August 18, 2016 16:05
@jrluis jrluis changed the title add support for cucumber 2.4.0 add support for registering runners Aug 18, 2016
require 'rrrspec/client/rspec_runner_factory'

module RRRSpec
Registry.register_runner_factory RRRSpec::Client::RSpecRunnerFactory.new
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this to rspec_runner_factory.rb

Copy link
Author

Choose a reason for hiding this comment

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

done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants