Fix manual commits, add an option to rebuild any given commit [integr…

…ity#16 state:resolved]

app.rb

@@ -119,7 +119,7 @@
   login_required
 
   current_project.build
-  redirect project_url(@project)
+  redirect project_url(current_project)
 end
 
 get "/:project/commits/:commit" do
@@ -131,6 +131,13 @@
   redirect "/#{params[:project]}/commits/#{params[:commit]}", 301
 end
 
+post "/:project/commits/:commit/builds" do
+  login_required
+
+  current_project.build(params[:commit])
+  redirect commit_url(current_commit)
+end
+
 get "/integrity.css" do
   response["Content-Type"] = "text/css; charset=utf-8"
   sass :integrity

lib/integrity/author.rb

@@ -19,7 +19,7 @@ def to_s
     end
 
     def self.load(value, property)
-      AuthorStruct.parse(value)
+      AuthorStruct.parse(value) unless value.nil?
     end
 
     def self.dump(value, property)

lib/integrity/commit.rb

@@ -4,23 +4,31 @@ class Commit
 
     property :id,           Integer,  :serial => true
     property :identifier,   String,   :nullable => false
-    property :message,      String,   :nullable => false, :length => 255
-    property :author,       Author,   :nullable => false, :length => 255
-    property :committed_at, DateTime, :nullable => false
+    property :message,      String,   :length => 255
+    property :author,       Author,   :length => 255
+    property :committed_at, DateTime
     property :created_at,   DateTime
     property :updated_at,   DateTime
 
-    has 1,     :build,   :class_name => "Integrity::Build"
+    has 1,     :build,   :class_name => "Integrity::Build", :order => [:created_at.desc]
     belongs_to :project, :class_name => "Integrity::Project"
 
+    def message
+      attribute_get(:message) || "<Commit message not loaded>"
+    end
+
+    def author
+      attribute_get(:author) || Author.load('<Commit author not loaded> <<Commit author not loaded>>', :author)
+    end
+
     def short_identifier
       identifier.to_s[0..6]
     end
 
     def status
       build.nil? ? :pending : build.status
     end
-
+    
     def successful?
       status == :success
     end
@@ -60,4 +68,4 @@ def queue_build
     alias :commit_message          :message
     alias :commited_at             :committed_at
   end
-end
+end

lib/integrity/helpers/urls.rb

@@ -35,13 +35,13 @@ def push_url_for(project)
         end.to_s
       end
 
-      def commit_path(commit)
-        project_path(commit.project, "commits", commit.identifier)
+      def commit_path(commit, *path)
+        project_path(commit.project, "commits", commit.identifier, *path)
       end
 
-      def build_path(build)
+      def build_path(build, *path)
         warn "#build_path is deprecated, use #commit_path instead"
-        commit_path build.commit
+        commit_path build.commit, *path
       end
 
       def commit_url(commit)

lib/integrity/notifier/base.rb

@@ -9,46 +9,56 @@ def self.to_haml
         raise NoMethodError, "you need to implement this method in your notifier"
       end
 
-      attr_reader :build
+      attr_reader :commit
 
-      def initialize(build, config)
-        @build = build
+      def initialize(commit, config)
+        @commit = commit
         @config = config
       end
 
+      def build
+        warn "Notifier::Base#build is deprecated, use Notifier::Base#commit instead"
+        commit
+      end
+
       def deliver!
         raise NoMethodError, "you need to implement this method in your notifier"
       end
 
       def short_message
-        "Build #{build.short_commit_identifier} #{build.successful? ? "was successful" : "failed"}"
+        commit.human_readable_status
       end
 
       def full_message
         <<-EOM
-"Build #{build.commit_identifier} #{build.successful? ? "was successful" : "failed"}"
+"Build #{commit.identifier} #{commit.successful? ? "was successful" : "failed"}"
 
-Commit Message: #{build.commit_message}
-Commit Date: #{build.commited_at}
-Commit Author: #{build.commit_author.name}
+Commit Message: #{commit.message}
+Commit Date: #{commit.committed_at}
+Commit Author: #{commit.author.name}
 
 Link: #{commit_url}
 
 Build Output:
 
-#{stripped_build_output}
+#{stripped_commit_output}
 EOM
       end
 
       def commit_url
         raise if Integrity.config[:base_uri].nil?
-        Integrity.config[:base_uri] / build.project.permalink / "commits" / build.commit.identifier
+        Integrity.config[:base_uri] / commit.project.permalink / "commits" / commit.identifier
       end
 
       private
 
+        def stripped_commit_output
+          commit.output.gsub("\e[0m", "").gsub(/\e\[3[1-7]m/, "")
+        end
+
         def stripped_build_output
-          build.output.gsub("\e[0m", "").gsub(/\e\[3[1-7]m/, "")
+          warn "Notifier::Base#stripped_build_output is deprecated, use Notifier::base#stripped_commit_output instead"
+          stripped_commit_output
         end
     end
   end

lib/integrity/project.rb

@@ -30,7 +30,8 @@ def self.only_public_unless(condition)
     end
 
     def build(commit_identifier="HEAD")
-      commit = commits.first(:identifier => commit_identifier, :project_id => id) || last_commit
+      commit_identifier = head_of_remote_repo if commit_identifier == "HEAD"
+      commit = find_or_create_commit_with_identifier(commit_identifier)
       commit.queue_build
     end
 
@@ -73,6 +74,10 @@ def previous_builds
     def status
       last_commit && last_commit.status
     end
+
+    def human_readable_status
+      last_commit && last_commit.human_readable_status
+    end
 
     def public=(flag)
       attribute_set(:public, case flag
@@ -95,6 +100,23 @@ def enable_notifiers(*args)
     end
 
     private
+      def find_or_create_commit_with_identifier(commit_identifier)
+        # We abuse +committed_at+ here setting it to Time.now because we use it
+        # to sort (for last_commit and previous_commits). I don't like this
+        # very much, but for now it's the only solution I can find.
+        #
+        # This also creates a dependency, as now we *always* have to update the
+        # +committed_at+ field after building to ensure the date is correct :(
+        #
+        # This might also make your commit listings a little jumpy, if some
+        # commits change place every time a build finishes =\
+        commits.first_or_create({ :identifier => commit_identifier, :project_id => id }, :committed_at => Time.now)
+      end
+
+      def head_of_remote_repo
+        SCM.new(uri, branch).head
+      end
+
       def create_commit_from(data)
         commits.create(:identifier   => data["id"],
                        :author       => data["author"],

lib/integrity/project_builder.rb

@@ -17,6 +17,7 @@ def build(commit)
       @build
     ensure
       @build.update_attributes(:commit_id => commit.id, :completed_at => Time.now)
+      @commit.update_attributes(@scm.info(commit.identifier))
       send_notifications
     end
 
@@ -52,4 +53,4 @@ def run_build_script
         @build.successful = $?.success?
       end
   end
-end
+end

lib/integrity/scm.rb

@@ -16,4 +16,4 @@ def self.scm_class_for(uri)
         raise SCMUnknownError, "could not find any SCM based on URI '#{uri.to_s}'"
       end
   end
-end
+end

lib/integrity/scm/git.rb

@@ -9,7 +9,7 @@ def self.working_tree_path(uri)
         Git::URI.new(uri).working_tree_path
       end
 
-      def initialize(uri, branch, working_directory)
+      def initialize(uri, branch, working_directory=nil)
         @uri = uri.to_s
         @branch = branch.to_s
         @working_directory = working_directory
@@ -24,6 +24,16 @@ def with_revision(revision)
       def name
         self.class.name.split("::").last
       end
+
+      def head
+        log "Getting the HEAD of '#{uri}' at '#{branch}'"
+        `git ls-remote --heads #{uri} #{branch} | awk '{print $1}'`.chomp
+      end
+
+      def info(revision)
+        format  = %Q(---%n:author: %an <%ae>%n:message: >-%n  %s%n:committed_at: %ci%n)
+        YAML.load(`cd #{working_directory} && git show -s --pretty=format:"#{format}" #{revision}`)
+      end
 
       private
 
@@ -71,4 +81,4 @@ def log(message)
         end
     end
   end
-end
+end

test/acceptance/browse_project_test.rb

@@ -35,8 +35,8 @@ class BrowsePublicProjectsTest < Test::Unit::AcceptanceTestCase
 
     visit "/"
 
-    response_body.should =~ /Built #{integrity.last_commit.short_identifier} on Dec 15th successfully/m
-    response_body.should =~ /Built #{test.last_commit.short_identifier} on Dec 15th and failed/m
+    response_body.should =~ /Built #{integrity.last_commit.short_identifier} successfully/m
+    response_body.should =~ /Built #{test.last_commit.short_identifier} and failed/m
     response_body.should =~ /Never built yet/
     response_body.should =~ /Building!/
   end

test/acceptance/build_notifications_test.rb

@@ -9,8 +9,6 @@ class BuildNotificationsTest < Test::Unit::AcceptanceTestCase
   EOS
 
   scenario "an admin sets up a notifier for a project that didn't have any" do
-    return pending "Manual building is broken"
-
     git_repo(:my_test_project).add_successful_commit
     Project.gen(:my_test_project, :notifiers => [], :uri => git_repo(:my_test_project).path)
     rm_f "/tmp/textfile_notifications.txt"
@@ -27,9 +25,9 @@ class BuildNotificationsTest < Test::Unit::AcceptanceTestCase
     click_button "manual build"
 
     notification = File.read("/tmp/textfile_notifications.txt")
-    notification.should =~ /=== Build #{git_repo(:my_test_project).short_head} was successful ===/
+    notification.should =~ /=== Built #{git_repo(:my_test_project).short_head} successfully ===/
     notification.should =~ /Build #{git_repo(:my_test_project).head} was successful/
-    notification.should =~ %r(http://integrity.example.org/my-test-project/builds/#{git_repo(:my_test_project).head})
+    notification.should =~ %r(http://integrity.example.org/my-test-project/commits/#{git_repo(:my_test_project).head})
     notification.should =~ /Commit Author: John Doe/
     notification.should =~ /Commit Date: (.+)/
     notification.should =~ /Commit Message: This commit will work/

test/acceptance/manual_build_project_test.rb

@@ -4,12 +4,10 @@ class ManualBuildProjectTest < Test::Unit::AcceptanceTestCase
   story <<-EOS
     As an administrator,
     I want to manually build my project
-    So that I know if it build properly
+    So that I know if it builds properly
   EOS
 
   scenario "clicking on 'Manual Build' triggers a successful build" do
-    return pending "Manual building is broken"
-
     git_repo(:my_test_project).add_successful_commit
     Project.gen(:my_test_project, :uri => git_repo(:my_test_project).path)
     login_as "admin", "test"
@@ -19,14 +17,12 @@ class ManualBuildProjectTest < Test::Unit::AcceptanceTestCase
 
     response_body.should have_tag("h1", /Built #{git_repo(:my_test_project).short_head} successfully/)
     response_body.should have_tag("blockquote p", /This commit will work/)  # commit message
-    response_body.should have_tag("span.who",     /by: John Doe/)         # commit author
+    response_body.should have_tag("span.who",     /by: John Doe/)           # commit author
     response_body.should have_tag("span.when",    /today/)                  # commit date
     response_body.should have_tag("pre.output",   /Running tests.../)       # build output
   end
-
+  
   scenario "clicking on 'Manual Build' triggers a failed build" do
-    return pending "Manual building is broken"
-
     git_repo(:my_test_project).add_failing_commit
     Project.gen(:my_test_project, :uri => git_repo(:my_test_project).path)
     login_as "admin", "test"
@@ -39,11 +35,11 @@ class ManualBuildProjectTest < Test::Unit::AcceptanceTestCase
   end
 
   scenario "fixing the build command and then rebuilding result in a successful build" do
-    return pending "Manual building is broken"
-
     git_repo(:my_test_project).add_successful_commit
-    project = Project.gen(:my_test_project, :uri => git_repo(:my_test_project).path,
-                          :command => "ruby not-found.rb")
+    Project.gen(:my_test_project, 
+                :uri => git_repo(:my_test_project).path,
+                :command => "ruby not-found.rb")
+
     login_as "admin", "test"
 
     visit "/my-test-project"
@@ -55,8 +51,30 @@ class ManualBuildProjectTest < Test::Unit::AcceptanceTestCase
     click_button "Update Project"
 
     visit "/my-test-project"
-    click_button "Request Manual Build"
+    click_button "Build the last commit"
 
     response_body.should have_tag("h1", /success/)
   end
+
+  scenario "Successful builds should not display the 'Rebuild' button" do
+    git_repo(:my_test_project).add_successful_commit
+    Project.gen(:my_test_project, :uri => git_repo(:my_test_project).path)
+    login_as "admin", "test"
+
+    visit "/my-test-project"
+    click_button "manual build"
+
+    response_body.should_not have_tag("button", "Rebuild")
+  end
+
+  scenario "Failed builds should display the 'Rebuild' button" do
+    git_repo(:my_test_project).add_failing_commit
+    Project.gen(:my_test_project, :uri => git_repo(:my_test_project).path)
+    login_as "admin", "test"
+
+    visit "/my-test-project"
+    click_button "manual build"
+
+    response_body.should have_tag("button", "Rebuild")
+  end
 end

test/helpers/fixtures.rb

@@ -54,7 +54,7 @@ def deliver!;     nil;  end
   { :identifier =>   Digest::SHA1.hexdigest(/[:paragraph:]/.gen),
     :message =>      /[:sentence:]/.gen,
     :author =>       /\w+ \w+ <\w+@example.org>/.gen,
-    :committed_at => unique {|i| Time.mktime(2009, 12, 15, 18, (59 - i) % 60) },
+    :committed_at => unique {|i| Time.mktime(2008, 12, 15, 18, (59 - i) % 60) },
     :project_id =>   project.id }
 end