Use Kumade::CommandLine to run commands. #63

Merged
merged 2 commits into from Sep 23, 2011

Projects

None yet

4 participants

@gabebw
Member
gabebw commented Sep 23, 2011

This commit also gives full test coverage to the Git class.

@gabebw gabebw Use Kumade::CommandLine to run commands.
This commit also gives full test coverage to the Git class.
72424c0
@qrush qrush commented on the diff Sep 23, 2011
lib/kumade/command_line.rb
@@ -0,0 +1,41 @@
+require 'cocaine'
qrush
qrush Sep 23, 2011

do this in lib/kumade.rb please

joshuaclayton
joshuaclayton Sep 23, 2011 Owner

Why there? I like keeping the requires where they're actually needed...

@qrush qrush commented on the diff Sep 23, 2011
spec/support/git.rb
@@ -0,0 +1,5 @@
+module GitHelpers
+ def dirty_the_repo
+ `echo dirty_it_up > .gitkeep`
qrush
qrush Sep 23, 2011

how about just touch .gitkeep ?

gabebw
gabebw Sep 23, 2011 Member

.gitkeep is created in an around block in spec_helper, so that each test runs in its own git repo. We need to actually change the content of a file that's already in the repo history, and this is the only one that fits the bill.

@mike-burns mike-burns commented on the diff Sep 23, 2011
spec/kumade/command_line_spec.rb
@@ -0,0 +1,109 @@
+require 'spec_helper'
+
+describe Kumade::CommandLine, "#run_or_error" do
+ let(:command_line) { stub("Cocaine::CommandLine instance", :run => nil) }
+ subject { Kumade::CommandLine.new("echo") }
+
+ before do
+ subject.stubs(:error)
+ subject.stubs(:say_status)
+
+ Cocaine::CommandLine.stubs(:new).returns(command_line)
mike-burns
mike-burns Sep 23, 2011 Owner

You could have instead used DI. Just sayin'.

@mike-burns mike-burns and 1 other commented on an outdated diff Sep 23, 2011
lib/kumade/command_line.rb
+module Kumade
+ class CommandFailedError < RuntimeError
+ end
+
+ class CommandLine < Base
+ def initialize(command_to_run)
+ super()
+ @command_line = Cocaine::CommandLine.new(command_to_run)
+ end
+
+ def run_or_error(error_message = nil)
+ if run_with_status
+ true
+ else
+ error(error_message)
+ end
mike-burns
mike-burns Sep 23, 2011 Owner

How do you feel about:

run_with_status || error(error_message)
gabebw
gabebw Sep 23, 2011 Member

I like it!

@qrush qrush commented on the diff Sep 23, 2011
spec/kumade/git_spec.rb
+ it "switches to the given branch" do
+ subject.add_and_commit_all_in(directory, branch, commit_message, 'success', 'error')
+ subject.current_branch.should == branch
+ end
+
+ it "uses the given commit message" do
+ subject.add_and_commit_all_in(directory, branch, commit_message, 'success', 'error')
+ `git log -n1 --pretty=format:%s`.should == commit_message
qrush
qrush Sep 23, 2011

this is probably nitpicky, not sure if this is better:

`git log --oneline -1`.should include(commit_message)
@mike-burns mike-burns and 1 other commented on an outdated diff Sep 23, 2011
lib/kumade/command_line.rb
@@ -0,0 +1,41 @@
+require 'cocaine'
+
+module Kumade
+ class CommandFailedError < RuntimeError
mike-burns
mike-burns Sep 23, 2011 Owner

Where is this used?

gabebw
gabebw Sep 23, 2011 Member

Oh, that's left over from earlier code. Oops.

@mike-burns mike-burns commented on the diff Sep 23, 2011
lib/kumade/git.rb
@@ -20,25 +20,27 @@ module Kumade
mike-burns
mike-burns Sep 23, 2011 Owner

If this constructor took a command line builder object, you could test that command line builder separately, reducing writes to the FS and IO, and also pass a mock command line builder object that has RSpec matchers for #has_pushed_new_instance? and so on.

This mock command line builder would also make it easier for other people to test derived implementations, if they make such a thing.

gabebw
gabebw Sep 23, 2011 Member

Like this?

Git.new(command_line_builder = Kumade::CommandLineBuilder)

then inside Git:

command = command_line_builder.add("git push").add("-f" if force).add(remote).add(branch).build
mike-burns
mike-burns Sep 23, 2011 Owner

Right, and then in your tests:

let(:command_line_builder) { MockCommandLineBuilder.new }
subject { Git.new(:command_line_builder => command_line_builder) }

it "pushes to the correct remote" do
  subject.push(branch, remote)
  command_build_builder.should have_pushed(branch, remote)
end
@gabebw gabebw merged commit 0c3840d into master Sep 23, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment