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

Allow to force the test framework and the application init file on the command line #5

Closed
wants to merge 2 commits into from

Conversation

mathieul
Copy link

@mathieul mathieul commented Nov 1, 2011

Thanks for a great tool that already speeds up our TDD workflow!

I wanted to use it for 2 different apps, one using rails 2.3 and another one using 3.0. Also we're using RSpec inside our Cucumber scenarios, but are using Test unit for all other tests.

So I added an option to allow to force the test framework:

$ spin serve -t test

And another one to specify the application init file (config/application.rb for rails 3, config/environment.rb for rails 2 for instance):

$ spin serve -i config/environment.rb

As part of those modifications I refactored the code to structure it into classes (personal preference, makes it easier to extend) and am using OptParser standard library to parse the command line.

Thanks again, hopefully you'll find this pull request useful.

@jstorimer
Copy link
Owner

Hey Mathieu,

Glad it's working for you!

I incorporated the spirit behind your -t option as the --rspec option. I incorporated part of your optparse changes so now you can see the options supported with:

spin -h

I chose not to split everything into classes (yet). At this point in time I think it just adds more complexity to the project without much gain. If the codebase keeps growing then I'll probably split everything up as you did and put them in their own files under lib/.

I didn't incorporate your -i argument suggestion because you really don't want to load config/environment.rb for Rails 2. Doing so will preload all of your app code (models, controllers). So if you make any changes to your app and try to run tests with spin you'll be running tests against old code and unexpected results will ensue. Definitely don't want that. I don't do much with Rails 2 anymore but if we can come up with a working solution I'd be happy to include it.

Thanks for your hard work!

Jesse

@jstorimer jstorimer closed this Nov 2, 2011
@mathieul
Copy link
Author

mathieul commented Nov 2, 2011

Hey Jesse,

I understand you don't want to complicate your code, and rocco helps easily go through it anyway.

Talking about that I should have spent more time reading your code and comments, and would have understood then that loading environment.rb was not a good idea… Sorry about that.

Thanks for pointing it out. I made a few tests this morning with our 2.3 app. I loaded config/boot.rb instead of the environment.rb, and:

- ran several times one of our unit test that takes 51 seconds when ran through "rake test:units TEST=test/..." (it's a big application, lots of legacy code, running tests takes forever…)
- ran it several time using guard (and the guard-shell plugin to spawn spin push when an application file or a test file is updated), and it then "only" takes 40 seconds

The gain is not huge, but it is still 20 % gain (that might be an extreme though, as the test file doesn't test much, so most it is spent in initialization - more tests would likely reduce the gain %).

Thanks again for spin, cheers!

-- Mathieu

On Tuesday, November 1, 2011 at 10:00 PM, Jesse Storimer wrote:

Hey Mathieu,

Glad it's working for you!

I incorporated the spirit behind your -t option as the --rspec option. I incorporated part of your optparse changes so now you can see the options supported with:

spin -h

I chose not to split everything into classes (yet). At this point in time I think it just adds more complexity to the project without much gain. If the codebase keeps growing then I'll probably split everything up as you did and put them in their own files under lib/.

I didn't incorporate your -i argument suggestion because you really don't want to load config/environment.rb for Rails 2. Doing so will preload all of your app code (models, controllers). So if you make any changes to your app and try to run tests with spin you'll be running tests against old code and unexpected results will ensue. Definitely don't want that. I don't do much with Rails 2 anymore but if we can come up with a working solution I'd be happy to include it.

Thanks for your hard work!

Jesse

Reply to this email directly or view it on GitHub:
#5 (comment)

@jstorimer
Copy link
Owner

Thanks for doing that research, that sounds promising. What does config/boot.rb look like in a Rails 2.3 app? Can you gist it?

@mathieul
Copy link
Author

mathieul commented Nov 3, 2011

Hi Jesse,

Here you go: https://gist.github.com/1337051

-- Mathieu

On Wednesday, November 2, 2011 at 9:35 PM, Jesse Storimer wrote:

Thanks for doing that research, that sounds promising. What does config/boot.rb look like in a Rails 2.3 app? Can you gist it?

Reply to this email directly or view it on GitHub:
#5 (comment)

@jstorimer
Copy link
Owner

Thanks for that :) That file is such a hack! But it looks like it should work for our purposes. It will just preload Rails, and not the gem deps, but it looks like those are tightly coupled to the app initialization in 2.x.

I pushed some code to the jstorimer@rails2 branch (b338952) that should achieve the desired effect. Can you give it a try with your 2.x app?

@jstorimer jstorimer mentioned this pull request Nov 5, 2011
@grosser
Copy link
Collaborator

grosser commented Jul 15, 2012

atm in our rails 2 app we use a custom crappy thing to load our config/environment.rb and then fork of, it's not perfect, but it beats waiting 20s for a test run. This pull would be a great help in getting something a bit better, and it's a nice cleanup of the codebase either way

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.

None yet

3 participants