Specs #4

Merged
merged 4 commits into from Apr 3, 2013

2 participants

@linedotstar

@jcwilk can you take a look at the specs? it's a little awkward spec'ing capistrano tasks. any ideas on how to make these cleaner would be appreciated.

Chris Friedrich and others added some commits Apr 2, 2013
@jcwilk jcwilk commented on the diff Apr 3, 2013
spec/campystrano_spec.rb
@@ -0,0 +1,107 @@
+require 'spec_helper'
+
+# include #_cset since it's not automatically included in Capistrano::Configuration
@jcwilk
jcwilk added a note Apr 3, 2013

is there no way to require some file or something like that so that the definition comes from capistrano code rather than duplicating it here?

Unfortunately, they define this method in the deploy recipe: https://github.com/capistrano/capistrano/blob/master/lib/capistrano/recipes/deploy.rb#L8
which includes a lot of other code that I didn't want to introduce into the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jcwilk jcwilk and 1 other commented on an outdated diff Apr 3, 2013
spec/campystrano_spec.rb
+ c.set(:application) { application }
+ c.set(:rails_env) { rails_env }
+ c.set(:branch) { branch }
+ c.set(:campfire_emoji) { campfire_emoji }
+ c.set(:campfire_settings) { campfire_settings }
+
+ Tinder::Campfire.stub(:new).and_return(campfire)
+ c.stub(:`).with('whoami').and_return(user)
+
+ Capistrano::Campystrano.load_into(c)
+ end
+ end
+
+ subject { config.find_and_execute_task(task) }
+
+ shared_examples_for 'a campystrano deploy task' do |message|
@jcwilk
jcwilk added a note Apr 3, 2013

is |message| supposed to be here? looks like it's not used

gc. fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jcwilk jcwilk commented on the diff Apr 3, 2013
spec/campystrano_spec.rb
@@ -0,0 +1,107 @@
+require 'spec_helper'
+
+# include #_cset since it's not automatically included in Capistrano::Configuration
+class Capistrano::Configuration
+ def _cset(name, *args, &block)
+ unless exists?(name)
+ set(name, *args, &block)
+ end
+ end
+end
+
+describe Capistrano::Campystrano do
@jcwilk
jcwilk added a note Apr 3, 2013

Also, does this have to be under the Capistrano namespace? I'm not really familiar with standards around external deploy task definitions and such, but seems like it ought to just be Campystrano in the absence of a conflicting standard.

it probably doesn't. but i'd rather tackle that as part of a separate PR

@jcwilk
jcwilk added a note Apr 3, 2013

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jcwilk

Looks reasonable. Seems like you're adequately locking down the behavior defined in the code and it's pretty legible. Nice work :shipit:

Chris Friedrich version bump 7b91864
@linedotstar linedotstar merged commit 16c073c into master Apr 3, 2013
@linedotstar linedotstar deleted the specs branch Apr 3, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment