Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Respect minitest/pride if it was loaded. #5

Open
wants to merge 2 commits into from

7 participants

@rmm5t rmm5t referenced this pull request in guard/guard-minitest
Merged

Simplifying the DRb version of the runner #41

@rmm5t

+1 This is much needed, especially adding support for the command line arguments. A must if you also use guard-minitest.

@semaperepelitsa

I want this project to be a very thin layer above Minitest and this is a key difference from spork-testunit. Guard-minitest originally targeted spork-testunit hence it used these options. But they are not really doing anything useful except configuring Growl. It can be set up in the test helper as fine.

I think guard-minitest should just omit these options if notification is disabled and the issue is gone.

if notify?
  cmd_parts << "-r #{File.expand_path('../runners/default_runner.rb', __FILE__)}"
  cmd_parts << "-e '::GUARD_NOTIFY=#{notify?}'"
end
@semaperepelitsa

And I don't have a solution for minitest/pride. Certainly, I'm not bringing this mess here. (Yes, I'm aware that this was my pull request :-)

@rmm5t

Disabling the ability to do notifications with guard seems to completely miss the point of using guard in the first place. Notifications are what make Guard so useful.

While, I'd prefer to see these testdrb arguments parsed by OptionParser (require "optparse"), @nuxill's first stab at this enables spork-minitest to be used with guard-minitest. While I understand the desire to keep a very thin layer above Minitest, I don't think it's such a bad thing to enable some parity with spork-testunit to ensure a level of functionality that allows spork-minitest to be used in cool ways.

Let's be honest too. While spork is a great thing, tools like guard are really what make spork shine.

Aside, what was wrong with @nuxlli's solution for supporting minitest/pride?

@semaperepelitsa

I'm totally not against notifications, they are great. What I was trying to say is that you can setup them in the test_helper directly without those clunky options passed to testdrb:

require "guard/minitest/runners/default_runner"
GUARD_NOTIFY = true

Spork-testunit parsed all options itself and did not pass them to MiniTest. I don't want that. I want all options to be passed through to MiniTest Unit runner and let it interpret them. This is a feature.

You can write a custom MiniTest::Unit subclass to support any additional options but this does not belong here.

Having said that, guard-minitest integration would be very nice. But I think it should be done in a different way.

@rmm5t

@semaperepelitsa Fair arguments. In the end, I'd really like to see guard-mintest and spork-minitest play nice together. Right now, they do not.

It would also be nice if guard-minitest and spork-minitest could co-exist without having to add a few lines of configuration to every project that use both of them, but getting the two working together at all would be an excellent first step. I think we agree that guard-minitest could use some cleanup.

@sbleon

This pull request on guard-minitest makes it compatible with spork, but does break notifications.
guard/guard-minitest#42

Please +1 it if it solves this problem for you, or suggest some modifications to keep notifications working if that's important to you.

@zenspider

"Spork-testunit parsed all options itself and did not pass them to MiniTest. I don't want that. I want all options to be passed through to MiniTest Unit runner and let it interpret them. This is a feature."

I think this is the crux of the issue and certainly needs to be addressed.

@semaperepelitsa

@zenspider what do you suggest?

@naomik

Thanks you so much for this patch.

I'm extremely irritated the the original gem maintainer has not added this code; especially because it seems like he has a really bad attitude about it.

@martco

@nuxlli thank you for this. it's a shame that @semaperepelitsa seems to be sucking his thumb on this one. i'm going to try it out locally regardless. cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 16, 2012
  1. @nuxlli
  2. @nuxlli

    Added support for -r, -e and -I

    nuxlli authored
This page is out of date. Refresh to see the latest.
Showing with 44 additions and 15 deletions.
  1. +44 −15 lib/spork/test_framework/minitest.rb
View
59 lib/spork/test_framework/minitest.rb
@@ -5,22 +5,51 @@ class Spork::TestFramework::MiniTest < Spork::TestFramework
def run_tests(argv, stderr, stdout)
require "minitest/unit"
$LOAD_PATH << "test" << "."
- ::MiniTest::Unit.output = stdout
-
- paths, opts = parse_options(argv)
-
- paths.each do |path|
- require path
+
+ io_class = ::MiniTest::Unit.output.class
+ if defined?(PrideIO) and io_class == PrideIO or defined?(PrideLOL) and io_class == PrideLOL
+ # Respect minitest/pride
+ ::MiniTest::Unit.output = io_class.new(stdout)
+ else
+ ::MiniTest::Unit.output = stdout
end
+
+ # MiniTest's test/unit does not support -I, -r, or -e
+ # Extract them and remove from arguments that are passed to testrb.
+ argv.each_with_index do |arg, idx|
+ if arg =~ /-I(.*)/
+ if $1 == ''
+ # Path is next argument.
+ include_path = argv[idx + 1]
+ argv[idx + 1] = nil # Will be squashed when compact called.
+ else
+ include_path = $1
+ end
+ $LOAD_PATH << include_path
+ argv[idx] = nil
+ elsif arg =~ /-r(.*)/
+ if $1 == ''
+ # File is next argument.
+ require_file = argv[idx + 1]
+ argv[idx + 1] = nil # Will be squashed when compact called.
+ else
+ require_file = $1
+ end
+ require require_file
+ argv[idx] = nil
+ elsif arg =~ /^-e$/
+ eval argv[idx + 1]
+ argv[idx] = argv[idx + 1] = nil
+ elsif arg == '--'
+ argv[idx] = nil
+ break
+ elsif !arg.nil?
+ require arg
+ argv[idx] = nil
+ end
+ end
+ argv.compact!
- ::MiniTest::Unit.new.run(opts)
- end
-
- def parse_options(argv)
- paths, opts = argv.slice_before("--").to_a
- paths ||= []
- opts ||= []
- opts.shift
- [paths, opts]
+ ::MiniTest::Unit.new.run(argv)
end
end
Something went wrong with that request. Please try again.