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

New options for run_all/reload guard method #14

Closed
thibaudgg opened this issue Oct 31, 2010 · 21 comments
Closed

New options for run_all/reload guard method #14

thibaudgg opened this issue Oct 31, 2010 · 21 comments

Comments

@thibaudgg
Copy link
Member

Discussed with YannLugrin, three new options should be added for run_all/reload guard method and could be specify per guard. Options are:

  • each => "5.minutes" (run the method each X minutes/seconds)
  • at_start => true/false (run the method when starting guard, false by default)
  • disabled => true/false (avoid to run the method when signal sent, false by default)

Example:

guard 'rspec', :run_all => { :at_start => true } do ...

What do you think about that?

@yannlugrin
Copy link
Member

I totally agree ;)

I push a "working in progress" feature branch today or tomorrow.

@rymai
Copy link
Member

rymai commented Nov 1, 2010

These options seem cool to me but will it be possible for guards developer to set those without the possibility for users to overwrite these options?
In other words, who will have the priority, guards developers or guards users?

For example, if the developer of a guard wants to never reload or run_all on any signals, the user should not be able to specify options that would cancel this behavior.

@thibaudgg
Copy link
Member Author

If a developer never want to reload/run_all on signal, actually the only way to call these methods, he just don't implement them :-)
Guard users will always have the priority.

@rymai
Copy link
Member

rymai commented Nov 2, 2010

Ok fair enough, btw my example was the opposite of what I thought: "If a developer wants to always run_all/reload, the user should not be able to cancel this behavior".

Do you have some concrete examples of the use of these options in a "real scenario"?

@thibaudgg
Copy link
Member Author

In my opinion if run_all/reload should not be disabled the user need to be smart enough to not doing that.

"Real scenario" Examples:

  • Running all spesc when guard starts
  • Not running all cucumber features with Ctrl-\

@yannlugrin
Copy link
Member

You can see work in progress branch at https://github.com/guard/guard/tree/guard_optionnal_actions

:disable and :at_start work

with this implementation you can control this in each guard code to add more optional case (but it is not recommended to overwrite normal option result).

@netzpirat
Copy link
Contributor

I like the new options and I'd like to see even more of these: it would be nice if I could make the execution of a guard dependent on the result of another guard.

Let me illustrate this with a few examples:

guard 'rspec' do
   ...
end

guard 'cucumber', :run_all => { :depends => :rspec }  true do
   ...
end

This configuration will run all cucumber features after rspec has run all specs successfully. This would enable a autotest like configuration for guard-rspec and guard-cucumber.

guard 'coffeescript' do
   ...
end

guard 'jasmine', :run_on_change => { :depends => :coffeescript } do
   ...
end

This configuration would run jasmine tests for a changed file after is has been successfully compiled into JavaScript. (The Jasmine guard doesn't exist for now, but who know).

If there are multiple guards of the same type configured, they can have an id options for unique identification:

guard 'coffeescript', :id => :coffee_app do
   ... watch app ...
end

guard 'coffeescript', :id => :coffee_tests do
   ... watch tests ...
end

guard 'jasmine', :run_on_change => { 
    :depends => [:coffee_app, :coffee_tests] 
} do
   ...
end

This implies a change that each plugin has to return either true or false in the #run_on_change and #run_all methods.

Michael

@thibaudgg
Copy link
Member Author

Yes, :depends option is a good idea, but the :id stuff is maybe overkill... a smart convention could be better... don't have better idea at the moment :-)

I'm thinking about another option, something like :non_blocking/threadable => true, that will make running the guard plugin without having to wait for his result (could be useful for guard-passenger).

@netzpirat
Copy link
Contributor

Guard referencing by id may be unnecessary if we switch to a notification push approach instead of a pull approach.

Last week I sat down and wrote my dream Guardfile and it looks like this:

threads 4

group :autotest do
  guard 'rspec', :version => 2, :start => true, :rerun => true, :success => { :all => :cucumber } do
    watch('^spec/(.*)_spec.rb')
    watch('^spec/spec_helper.rb')                       { 'spec' }

    watch('^app/(.*)\.rb')                              { |m| "spec/#{m[1]}_spec.rb" }
    watch('^lib/(.*)\.rb')                              { |m| "spec/lib/#{m[1]}_spec.rb" }

    watch('^app/controllers/application_controller.rb') { 'spec/controllers' }
    watch('^config/routes.rb')                          { 'spec/routing' }

    watch('^spec/fabricators/*')                        { 'spec/models' }
  end

  guard 'cucumber', :rerun => true do
    watch('^features/(.*).feature')
    watch('^features/support')                          { 'features' }
    watch('^features/step_definitions')                 { 'features' }
  end
end

group :frontend do
  guard 'compass' do
    watch('^app/stylesheets/(.*)\.scss')
  end

  guard 'coffeescript', :output => 'public/javascripts/compiled' do
    watch('^app/coffeescripts/(.*)\.coffee')
  end

  guard 'coffeescript', :output => 'spec/javascripts' do
    watch('^spec/coffeescripts/(.*)\.coffee')
  end

  guard 'livereload' do
    watch('^app/.+\.(erb|haml)$')
    watch('^app/helpers/.+\.rb$')
    watch('^public/.+\.(css|js|html)$')
    watch('^config/locales/.+\.yml$')
    watch('^spec/javascripts/.+\.js$')
  end
end

Let me dissect the above Guardfile:

threads

The thread keyword switches the Guard loop to a non-blocking mode, where the fs listener will keep listening for changes and spawns the Guard executions to the maximum number of threads defined. This allows to choose the optimal setting depending on the CPU.

group

The above example introduces groups that let me choose to run only a subset of the guardfile:

guard                        # Run all groups
guard autotest               # Run only the autotest group
guard frontend               # Run only the frontend group
guard autotest frontend      # Run both the autotest and the frontend group

So when I am focusing on frontend development, I could omit the whole backend testing stuff.

:start option

When the :start option is set to true, Guard will trigger run_all upon start. It defaults to false.

:rerun option

When the :rerun option is set, Guard will trigger run_on_change with all the failed files from the previous run if any of the guarded files (of the actual guard) is changed. It defaults to false.

This would imply that a Guard must return an Array with the failed files after a run method has been triggered. If everything succeed returns nil or an empty Array.

:success option

With the :success option a Guard can trigger other Guards run methods after it ran successfully.

The :success option takes an option Hash with two possible keys: :all will trigger run_all whereas :changed will trigger :run_on_change on the specified Guard(s). The value is either a single symbol of the destination Guard or an Array with multiple destination Guards.

What do you guys think of this DSL extensions? Do you see any problems? I'm wondering especially about the thread option, since I have no clue why it was decided to block the fs listener on guard execution.

The :start, :rerun and :success options will allow us to build more complex Guard configuration, for example to achieve an Autotest like behavior. Do you would like to see this implemented or do you think this is to heavy weight for Guard?

Thanks for any feedback.

@thibaudgg
Copy link
Member Author

Here some remarks for each points:

Threads

I don't think all guard should be running in thread, for example guard-spork must always running before guard-rspec/cucumber. So there is some real dependencies between some guards. Maybe a :thread => true option is enough to make just some guard non-blocking. Setting the number of maximum threads seems overkill for me, but why not.

Groups

Sound like a great feature to add

:start option

It's the same as :run_all => { :at_start => true } no?

:rerun option

This option should be added inside guard-rspec/test/cucumber instead of Guard itself, don't you think?

:success option

I think using :run_all & :run_on_change directly will be more clear than :all & :changed. It'll give something like :success => { :run_all => :cucumber }.
Good idea.


I just commit a change that remove the blockage of listeners on guards execution so a :thread => true option should be really easy to implement now. May be we can speak a little more about :start => true vs :run_all => { :at_start => true } before beginning any implementation. After that any pull request are very welcome as my hobby coding time is really limited :-).

Thanks for all your suggestions/ideas.

@netzpirat
Copy link
Contributor

The change you made on Guard to not block the listeners is sufficient and makes the threading obsolete for me.

I will definitely take care of the Guard Groups and you can expect a pull request in the near future.

I also agree that the rerun option is very specific and should be handled in the Guard implementations that need such a feature. I will do this for guard-cucumber soon.

Regarding the automatic start of a Guard and triggering other Guards after completion I also think we need to find the optimal DSL before starting the implementation. But since yannlugrin already started a branch with :run_all => { :at_start => true } it may be a good idea to keep the syntax close to his implementation and use something like :on_success => { :run_all => :cucumber } for the automatic Guard triggering.

Thanks for the feedback.

@thecatwasnot
Copy link

I think this is going to end up just a +1 comment. But I'm happy everyone is thinking about this. Would love to plug :on_success => { :run_all => :cucumber, :run_all => :rspec} into my Guardfile spork config and have it automatically tell me what the hell I was breaking when I last shut this project down ;)
For my own small contribution, I'm finding the more verbose syntax a lot clearer.

@rymai
Copy link
Member

rymai commented Apr 20, 2011

Hi guys,

The group option has been implemented, the rerun option should be implemented by the guards who need it, for example guard-rspec (guard/guard-rspec@f241899), so all is left to do for the moment is the :success option.

Could we discuss it and maybe finally find a proper DSL for it? We should also define precisely all the possibility of this option.

Examples:

# will run the Guard::RSpec#run_all and Guard::Cucumber#run_all methods.
guard 'spork', :on_success => { :run_all => [:rspec, :cucumber] }

# will run the Guard::Cucumber#run_on_change method with the paths run by Guard::RSpec.
guard 'rspec', :on_success => { :run_on_change => :cucumber }

@netzpirat
Copy link
Contributor

+1 for the latest DSL proposal. It's perfectly self-describing.

@thibaudgg
Copy link
Member Author

+1 for the latest DSL proposal too. We can make it! :)

@suzi2000
Copy link

suzi2000 commented Aug 2, 2011

usually i dont like the rerun of all specs on success, as it takes a long time and stops me from quickly adding a new test and seeing it fail. But disabling it completely seems too drastic, so how about

guard 'spork', :on_success => { :run_all_when_5_minutes_idle => [:rspec, :cucumber] }
guard 'spork', :on_success => { :run_all_when_50_seconds_idle => :rspec }

5_minutes_idle would be method missing, so it adjusts the idle time based on the method name

@rymai
Copy link
Member

rymai commented Aug 3, 2011

Hi,

Personally, I would prefer something like this:

guard 'spork', :on_success => { :run_all => [:rspec, :cucumber], :if => { :idle_for => 5*60 } }

It's maybe a bit too verbose, but the point is that I don't really like putting arguments inside a Hash key.

Also note that we probably would't use method_missing here, a simple "run_all_when_50_seconds_idle"[/run_all_when_(\d+)_seconds_idle/, 1].to_i (or similar) would suffice. :)

@suzi2000
Copy link

suzi2000 commented Aug 4, 2011

Oh now i see, thanks for the tip, agreed :)

@rymai
Copy link
Member

rymai commented Aug 4, 2011

You're welcome. By the way, I think your suggestion in itself, to run all the specs only when Guard has been idle for a certain amount of time is interesting. But anyway, we'll first have to implement the basis of this new :on_success option before thinking about conditional executions etc.

@netzpirat
Copy link
Contributor

Oh my dear GitHub issue, I've looked at you many times, because you're still in open state. I've also read your completely at least three times, which takes ages... But nobody cares about you, not you, dear reader and recipient of the notification, not the creator of the issue, not anyone of the Guard maintainers, just nobody. There's no discussion since 10 months and nobody want to spend some time for you. In short: we don't need you. I'm sorry about you, but I'm going to close you now...

@rymai
Copy link
Member

rymai commented Oct 18, 2012

👍 ^^

Sent from my iPhone

On 19 oct. 2012, at 00:42, Michael Kessler notifications@github.com wrote:

Oh my dear GitHub issue, I've looked at you many times, because you're still in open state. I've also read your completely at least three times, which takes ages... But nobody cares about you, not you, dear reader and recipient of the notification, not the creator of the issue, not anyone of the Guard maintainers, just nobody. There's no discussion since 10 months and nobody want to spend some time for you. In short: we don't need you. I'm sorry about you, but I'm going to close you now...


Reply to this email directly or view it on GitHub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants