Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplifying the DRb version of the runner #41

Merged
merged 3 commits into from Feb 21, 2013

Conversation

Projects
None yet
6 participants
@chadoh
Copy link
Contributor

chadoh commented May 1, 2012

We're using guard-minitest with spork-minitest. Spork works when running just spork and then using testdrb to run individual files, but guard-minitest was broken. It kept saying

Running tests with args ["-r", "/Users/costro0001/.rvm/gems/ruby-1.9.2-p180-patched/gems/guard-minitest-0.5.0/lib/guard/minitest/runners/default_runner.rb", "-e
", "::GUARD_NOTIFY=true", "test/test_helper.rb", "./test/unit/raw_site_test.rb"]...
Exception encountered: #<LoadError: no such file to load -- -r>

It turns out that the implementation of testdrb in spork-minitest does not allow the -r or -e options in the way guard-minitest had been passing them in. Additionally, I found that the inclusion of test_helper/spec_helper was also unnecessary, as these files were already being included in the individual test that was being run.

I'm not sure why all of this extra machinery was in place. It seems strange that I just removed it. Let me know what you think.

Simplifying the DRb version of the runner so that it works with the t…
…estdrb implementation in spork-minitest (not sure what the other one was set up for)
@chadoh

This comment has been minimized.

Copy link
Contributor Author

chadoh commented May 1, 2012

Oh, and yes, I'm aware that I probably "broke" the Growl notifications as a consequence of this. My team had not been using that, anyhow, so we don't mind. I don't really have a whole lot of time to play around with it, so I just wanted to get it working quickly.

@rmm5t

This comment has been minimized.

Copy link
Contributor

rmm5t commented Sep 22, 2012

I think this pull-request over on spork-minitest might be a better solution to the underlying problem.

semaperepelitsa/spork-minitest#5 specifically nuxlli/spork-minitest@ac59be9

@rking rking referenced this pull request Dec 6, 2012

Closed

Timeout / DRb::DRbConnError: #7

@rking

This comment has been minimized.

Copy link

rking commented Dec 6, 2012

@rmm5t Why do we need those options supported?

The testrunner works fine without being explicitly listed, and the notification is configurable just fine through other means.

I vote 👍 on 5a7f3c1

@rmm5t

This comment has been minimized.

Copy link
Contributor

rmm5t commented Dec 6, 2012

@rking Originally, I felt this way, because this pull-request breaks notification support, but after the discussion over at semaperepelitsa/spork-minitest#5, I agree now that it would be better to just simplify guard-minitest as described in the pull-request here.

@zenspider

This comment has been minimized.

Copy link

zenspider commented Dec 6, 2012

I agree and I like this PR.

@sbleon

This comment has been minimized.

Copy link
Member

sbleon commented Dec 12, 2012

+1

@rmm5t

This comment has been minimized.

Copy link
Contributor

rmm5t commented Jan 4, 2013

Any chance of this PR getting pulled in anytime soon? I've confirmed that this change combined with spork-minitest 1.0.0.beta2 both make things a little more friendly between the two gems.

@rking

This comment has been minimized.

Copy link

rking commented Jan 4, 2013

I just got bored and pushed the guard-sporkminitest gem.

It doesn't even have a 'drb' option, or any other cruft.

I use it indirectly, by just saying:

Gemfile

group :development do gem 'working' end

Guardfile

require 'working/guard'

test/test_helper.rb

require 'working/test_helper'
# Spork.prefork doesn't like when this line is missing 

I'm ready to support this 100%, so let me know with Github Issues or whatever.

@rking

This comment has been minimized.

Copy link

rking commented Jan 4, 2013

Also I released working-rails, that uses guard-minispork for Rails stuff.

All three of these gems need polish, but they currently work for me.

@rymai

This comment has been minimized.

Copy link
Member

rymai commented Feb 20, 2013

Hi guys!

Thanks for the discussion, strangely I didn't receive any notification about it (not sure how notifications are received for organization's repos...)!

@chadoh Could you rebase on master so I can merge it in?

Thanks!

@rymai rymai referenced this pull request Feb 21, 2013

Closed

guard-spork fix #50

@chadoh

This comment has been minimized.

Copy link
Contributor Author

chadoh commented Feb 21, 2013

Haha, I'm pleased that my 10-month-old pull request will finally be merged in! Unfortunately, I no longer work at the organization in which I used this, and I no longer have access to the repo. (@chrisledet! You might be the most likely of all my @itrcdevs comrades to want to help. These folks need my pull request rebased in order to accept it.)

@rymai

This comment has been minimized.

Copy link
Member

rymai commented Feb 21, 2013

Ok cool! Or I can merge it locally (the downside it that the commit will be from me instead of you...)?

Chris Ledet
Merge branch 'master' into fix_for_spork-minitest
Conflicts:
	lib/guard/minitest/runner.rb
	spec/guard/minitest/runner_spec.rb
@chadoh

This comment has been minimized.

Copy link
Contributor Author

chadoh commented Feb 21, 2013

Haha, @chrisledet's merge undid all my changes. 😦 He's given me access to the repo again; I'll fix it up at some point today.

@chadoh

This comment has been minimized.

Copy link
Contributor Author

chadoh commented Feb 21, 2013

Ok, it's obviously been a while and I'm no longer using this, but it looks like I redid all the original changes after @chrisledet helpfully merged master in.

@rymai

This comment has been minimized.

Copy link
Member

rymai commented Feb 21, 2013

Awesome!

rymai added a commit that referenced this pull request Feb 21, 2013

Merge pull request #41 from itrcdevs/fix_for_spork-minitest
Simplifying the DRb version of the runner

@rymai rymai merged commit 3dbc1ce into guard:master Feb 21, 2013

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.