Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

In process running with DRB via an explicit flag for blazingly fast specs [Proof of concept] #104

Closed
wants to merge 4 commits into from

4 participants

@nevir

Note that this is a proof of concept; If you agree w/ this approach, I'll be glad to fill in tests/deal w/ different spec versions/etc.

The motivation behind this was to get rid of the ~1sec delay spent resolving the rspec bin & loading rspec/core and friends every time a file changes. If you're using DRB, we can skip all that without worrying about corrupting the test environment.

A watched spec notifies almost instantaneously now.

Change notes:

  • Add :drb => true to your rspec guard block. This will cause guard-rspec
    to switch over to a much faster in-process rspec runner that avoids
    shelling out on each run.

  • When :drb is active, :cli is ignored. You must use :args, which takes
    an array of command line arguments (split on space) instead of a single
    string. This allows for unambiguous argument parsing when passing
    directly to rspec.

  • :cli is deprecated in favor of :args, since it can be applied with :drb
    being either on or off.

@nevir nevir In process running with DRB via an explicit flag for blazingly fast s…
…pecs

* Add :drb => true to your rspec guard block.  This will cause guard-rspec
  to switch over to a much faster in-process rspec runner that avoids
  shelling out on each run.

* When :drb is active, :cli is ignored.  You must use :args, which takes
  an array of command line arguments (split on space) instead of a single
  string.  This allows for unambiguous argument parsing when passing
  directly to rspec.

* :cli is deprecated in favor of :args, since it can be applied with :drb
  being either on or off.
6ee5179
@thibaudgg
Owner

Is there no conflict with the --drb flag used for Spork?

@nevir

The guard config I'm using is:


guard "bundler" do
  watch("Gemfile")
  watch(/^.+\.gemspec/)
end

guard "spork" do
  watch("Gemfile")
  watch("Gemfile.lock")
  watch(".rspec")              { :rspec }
  watch("spec/spec_helper.rb") { :rspec }
end

guard "rspec", version: 2, drb: true do
  watch(%r{^spec/.+_spec\.rb$})
  watch("spec/spec_helper.rb") { "spec" }

  watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" }
end

It also runs great if I take out guard-spork and run spark directly. It doesn't deal with the no-DRB-server case yet, though

@rymai
Owner

Hi, thanks for the proof of concept. A few notes about it:

  • I like the :args options replacing the :cli option, but I'd suggest the array element should group the key and value (if any), e.g. args => ["-f nested", "--color", ...] instead of args => ["-f", "nested", "--color", ...];
  • why creating a new :drb option (actually it's not new, it was there before the :cli option was introduced since DRB can be activated through the --drb CLI flag) and not simply check if the --drb flag is present in the :args option?

I also have a probably naive question: why don't you also use the "requiring rspec/core and using ::RSpec::Core::Runner.run(args) instead of system('rspec ...')" approach for when DRB is not used?

@nevir

On :args, I'm starting to think it's better to just stick with :cli for ease of use. The problem with a grouped approach, or just parsing the :cli string, though, is that I have to deal with extracting things like "some string with spaces" into a single argument, rather than just splitting on whitespace (and have to do my best to emulate whatever shell semantics the user's used to).

It seems like it's probably worth the effort, though; really complex args (with different string delimeters/nesting/etc) are likely an edge case.

I have no good argument for the stand alone :drb flag, I'm down with just checking for the existence of --drb; sounds good to me!

An alternative idea for the argument parsing might be to provide the arguments as a hash (with some handy stringification of different value types

args: {format: :nested, pattern: ["foo/*", "bar/**"]}

Would turn into:

--format "nested" --pattern "foo/*" --pattern "bar/**"
@nevir

On your last question - it's unsafe to call ::RSpec::Core::Runner.run when DRB is off because the test environment is loaded into the same process as the guard runner - subsequent test runs will be running against stale requires of the user's code (unless they implement some sort of auto-reloading mechanism themselves)

@rymai
Owner

Ok, sorry I made a big refactoring yesterday, could you rebase your pull-request on current master?

Regarding the :cli option, I say let's keep it for the moment but maybe we can improve it by allowing the hash syntax you proposed, let's see what @thibaudgg think of it.

Thanks!

@CaseyLeask

I've currently forked the guard-rspec so I could get :cli options before anything else is called.
Defining environmental vars in the call let me do tiered rspec testing, cutting out rails when I'm doing unit testing etc.
Would anyone else be interested in this use case?

@rymai
Owner

Hi, thanks for the proposition!

Is it related to the current pull-request? If not, please feel free to open a new issue/pull-request.

In any case, could you explain a bit more the possible use cases and the solution you propose, with examples etc..?

Thanks!!

@CaseyLeask

Sure.

I'm using different spec_helper files to get faster feedback on running tests.
For unit tests
SPEC=unit rspec spec/unit
For model tests
SPEC=model rspec spec/model
For a full test suite
SPEC=full rspec spec/

I'm using the ideas and code from http://paul.annesley.cc/2012/03/fast-rspec-slash-rails-tiered-spec-helper-dot-rb/ by @pda

While Paul runs the tests manually, I combined the idea of a cut-down test environment with guard-rspec to get speedy feedback automatically.

My very simplistic implementation of this idea can be viewed at CaseyLeask@52b13cb

How I use this is by adding :prefix => "SPEC=unit" (for unit tests) to arguments passed to guard-rspec in the Guardfile and specifying each type of tests as a different guard definition.

@nevir

Sounds like it'd be helpful to have an option supporting env vars directly

@nevir nevir referenced this pull request
Merged

Direct DRb #107

@nevir

Moving over to #107

@nevir nevir closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 23, 2012
  1. @nevir

    In process running with DRB via an explicit flag for blazingly fast s…

    nevir authored
    …pecs
    
    * Add :drb => true to your rspec guard block.  This will cause guard-rspec
      to switch over to a much faster in-process rspec runner that avoids
      shelling out on each run.
    
    * When :drb is active, :cli is ignored.  You must use :args, which takes
      an array of command line arguments (split on space) instead of a single
      string.  This allows for unambiguous argument parsing when passing
      directly to rspec.
    
    * :cli is deprecated in favor of :args, since it can be applied with :drb
      being either on or off.
Commits on Mar 24, 2012
  1. @nevir

    Comment change

    nevir authored
  2. @nevir
  3. @nevir
This page is out of date. Refresh to see the latest.
Showing with 51 additions and 9 deletions.
  1. +2 −3 lib/guard/rspec/inspector.rb
  2. +49 −6 lib/guard/rspec/runner.rb
View
5 lib/guard/rspec/inspector.rb
@@ -42,15 +42,14 @@ def feature_file?(path)
def spec_folder?(path)
path.match(%r{^(#{spec_paths.join("|")})[^\.]*$})
- # path.match(%r{^spec[^\.]*$})
end
def spec_files
- @spec_files ||= spec_paths.collect { |path| Dir[File.join(path, "**/*/**", "*_spec.rb")] }.flatten
+ @spec_files ||= spec_paths.collect { |path| Dir[File.join(path, "**", "*_spec.rb")] }.flatten
end
def feature_files
- @feature_files ||= spec_paths.collect { |path| Dir[File.join(path, "**/*/**", "*.feature")] }.flatten
+ @feature_files ||= spec_paths.collect { |path| Dir[File.join(path, "**", "*.feature")] }.flatten
end
def clear_spec_files_list_after
View
55 lib/guard/rspec/runner.rb
@@ -7,21 +7,65 @@ def run(paths, options={})
return false if paths.empty?
message = options[:message] || "Running: #{paths.join(' ')}"
UI.info(message, :reset => true)
- system(rspec_command(paths, options))
+
+ if options[:drb]
+ result = run_drb_rspec(paths, options)
+ else
+ result = exec_rspec(paths, options)
+ end
if options[:notification] != false && !drb?(options) && failure_exit_code_supported?(options) && $? && !$?.success? && $?.exitstatus != failure_exit_code
Notifier.notify("Failed", :title => "RSpec results", :image => :failed, :priority => 2)
end
+ result
+ end
+
+ def exec_rspec(paths, options={})
+ system(rspec_command(paths, options))
+
$?.success?
end
+ def run_drb_rspec(paths, options={})
+ require "rspec/core"
+
+ args = rspec_arguments(paths, options)
+ args << "--drb" unless args.include? "--drb"
+ args.map! { |a| a.to_s }
+
+ exit_code = ::RSpec::Core::Runner.run(args)
+
+ # Successful?
+ exit_code != failure_exit_code
+ end
+
def set_rspec_version(options={})
@rspec_version = options[:version] || determine_rspec_version
end
private
+ def rspec_arguments(paths, options={})
+ args = []
+ args += options[:args] if options[:args]
+ args += ["-f", "progress"] if options[:cli].nil? || !options[:cli].split(/[\s=]/).any? { |w| %w[-f --format].include?(w) } || args.include?('-f') || args.include?('--format')
+
+ if options[:notification] != false
+ args += ["-r", "#{File.dirname(__FILE__)}/formatters/notification_#{rspec_class.downcase}.rb"]
+ if rspec_version == 1
+ args += ["-f", "Guard::RSpec::Formatter::Notification#{rspec_class}:/dev/null"]
+ else
+ args += ["-f", "Guard::RSpec::Formatter::Notification#{rspec_class}", "--out", "/dev/null"]
+ end
+ end
+
+ args += ["--failure-exit-code", failure_exit_code] if failure_exit_code_supported?(options)
+ args += paths
+
+ args
+ end
+
def rspec_command(paths, options={})
warn_deprectation(options)
@@ -30,10 +74,7 @@ def rspec_command(paths, options={})
cmd_parts << "bundle exec" if (bundler? && options[:binstubs] == true && options[:bundler] != false) || (bundler? && options[:bundler] != false)
cmd_parts << rspec_exec(options)
cmd_parts << options[:cli] if options[:cli]
- cmd_parts << "-f progress" if options[:cli].nil? || !options[:cli].split(/[\s=]/).any? { |w| %w[-f --format].include?(w) }
- cmd_parts << "-r #{File.dirname(__FILE__)}/formatters/notification_#{rspec_class.downcase}.rb -f Guard::RSpec::Formatter::Notification#{rspec_class}#{rspec_version == 1 ? ":" : " --out "}/dev/null" if options[:notification] != false
- cmd_parts << "--failure-exit-code #{failure_exit_code}" if failure_exit_code_supported?(options)
- cmd_parts << paths.join(' ')
+ cmd_parts += rspec_arguments(paths, options)
cmd_parts.join(' ')
end
@@ -94,10 +135,12 @@ def rspec_exec(options = {})
end
def warn_deprectation(options={})
+ UI.info %{DEPRECATION WARNING: The :cli option is deprecated. Please use an array via :args => ["-f", "nested", "--color", ...]} if options[:cli]
+
[:color, :drb, :fail_fast, [:formatter, "format"]].each do |option|
key, value = option.is_a?(Array) ? option : [option, option.to_s.gsub('_', '-')]
if options.key?(key)
- UI.info %{DEPRECATION WARNING: The :#{key} option is deprecated. Pass standard command line argument "--#{value}" to RSpec with the :cli option.}
+ UI.info %{DEPRECATION WARNING: The :#{key} option is deprecated. Pass standard command line argument "--#{value}" to RSpec with the :args option.}
end
end
end
Something went wrong with that request. Please try again.