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.

Sign up for GitHub

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

Jump to bottom

Added Padrino support by default #89

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Added Padrino support by default #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3385a09
Changed spin to support padrino with new default
Who828 Nov 17, 2013
1123d0c
Added proper Padrino support by default
Who828 Nov 17, 2013
763d8c7
Changed the README and bumped of the version
Who828 Nov 17, 2013
bdd9edf
Revert the preload path to config/application.rb
Who828 Nov 17, 2013
8f1aa04
Reverted the version change
Who828 Nov 17, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
## Version 0.7.1 (Aug 19, 2013)

* fixed capability with MRI 2.0 [Todd Mazierski and Jonathan del Strother]
* Added Padrino support

## Version 0.7.0 (Jul 23, 2013)

Expand Down
10 changes: 5 additions & 5 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ Spin

[![Build Status](https://travis-ci.org/jstorimer/spin.png)](https://travis-ci.org/jstorimer/spin)

Spin speeds up your Rails testing workflow.
Spin speeds up your Rails/Padrino testing workflow.

By preloading your Rails environment in one process and then using fork(2) for each test run you don't load the same code over and over and over...
By preloading your Rails/Padrino environment in one process and then using fork(2) for each test run you don't load the same code over and over and over...
Spin works with an autotest(ish) workflow.

Installation
Expand All @@ -17,15 +17,15 @@ Spin is available as a rubygem.
gem i spin
```

Spin is a tool for Rails 3 apps. It is compatible with the following testing libraries:
Spin is a tool for Rails 3/Padrino apps. It is compatible with the following testing libraries:

* any version of test/unit or MiniTest
* RSpec 2.x

Usage
=====

There are two components to Spin, a server and client. The server has to be running for anything interesting to happen. You can start the Spin server from your `Rails.root` with the following command:
There are two components to Spin, a server and client. The server has to be running for anything interesting to happen. You can start the Spin server from your root dir with the following command:

``` bash
spin serve
Expand Down Expand Up @@ -108,7 +108,7 @@ There's another project ([spork](https://github.com/sporkrb/spork)) that aims to

2. It's simple.

Spin should work out of the box with any Rails app. No custom configuration required.
Spin should work out of the box with any Rails/Padrino app. No custom configuration required.

3. It doesn't do any [crazy monkey patching](https://github.com/sporkrb/spork-rails/blob/master/lib/spork/app_framework/rails.rb#L43-80).

Expand Down
1 change: 1 addition & 0 deletions lib/spin.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module Spin
class << self
def serve(options)
ENV['RAILS_ENV'] = 'test' unless ENV['RAILS_ENV']
ENV['PADRINO_ENV'] = 'test' unless ENV['PADRINO_ENV']
Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like we set ENV['PADRINO_ENV'] here and then check if it's set in lib/spin/cli.rb. Are you sure that's exactly what we need?

Copy link
Author

Choose a reason for hiding this comment

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

I am really sorry for these types of mistakes. We can't really use ENV to determine its a Padrino app (Like I originally thought). I didn't catch this bug because I was only testing it out on a Padrino app :/

Since Spin's philosophy is to run outside of your current app, I think its best to add as an option like (--padrino) and based on that I can add these configs.

Please share your thoughts on this

Copy link
Author

Choose a reason for hiding this comment

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

We can also check if config/application.rb is there or not (Padrino doesn't have this file) and based on that determine if app is Padrino or Rails.


if root_path = rails_root(options[:preload])
Dir.chdir(root_path)
Expand Down
2 changes: 2 additions & 0 deletions lib/spin/cli.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ def run(argv)
:preload => "config/application.rb"
}

options[:preload] = "config/boot.rb" if ENV['PADRINO_ENV']
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure that we need duplication here?

Copy link
Author

Choose a reason for hiding this comment

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

Well I wanted to add the support out of box, instead of using --preload to point to "config/boot.rb" manually.

If you want to be more verbose, I can create a method which says "padrino_app?" then use this or use the Rails default path.

My only aim is to make is simple for people who are new to this, that's all.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean that we add config/boot.rb to :preload by default and then add it one more time for padrino.

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I am sorry.

I meant the default to be "config/application.rb". Let me revert it back to orignal line.


parser = OptionParser.new do |opts|
opts.banner = usage
opts.separator ""
Expand Down