brew-test-bot: more Jenkins fixes and cleanup.

* Remove the unused HTML and commenting features.
* Return a good status code.
* Print command output on failure.
* Test bottle installation.

4 files changed, 38 insertions, 214 deletions

diff --git a/Library/Contributions/cmds/brew-test-bot.rb b/Library/Contributions/cmds/brew-test-bot.rb
index 088aa4c69..03d42e4ee 100755
--- a/Library/Contributions/cmds/brew-test-bot.rb
+++ b/Library/Contributions/cmds/brew-test-bot.rb
@@ -3,9 +3,7 @@
 # Usage: brew test-bot [options...] <pull-request|formula>
 #
 # Options:
-# --log:          Writes log files under ./brewbot/
-# --html:         Writes html and log files under ./brewbot/
-# --comment:      Comment on the pull request
+# --keep-logs:    Write and keep log files under ./brewbot/
 # --cleanup:      Clean the Homebrew directory. Very dangerous. Use with care.
 # --skip-setup:   Don't check the local system is setup correctly.
 
@@ -27,11 +25,9 @@ class Step
     @status = :running
     @repository = HOMEBREW_REPOSITORY
     @test.steps << self
-    write_html
   end
 
   def log_file_path full_path=true
-    return "/dev/null" unless ARGV.include? "--log" or ARGV.include? "--html"
     file = "#{@category}.#{@name}.txt"
     return file unless @test.log_root and full_path
     @test.log_root + file
@@ -60,23 +56,11 @@ class Step
     puts "#{Tty.send status_colour}#{status_upcase}#{Tty.reset}"
   end
 
-  def write_html
-    return unless @test.log_root and ARGV.include? "--html"
-
-    open(@test.log_root + "index.html", "w") do |index|
-      commit_html, css = @test.commit_html_and_css
-      index.write commit_html.result binding
-    end
-  end
-
-  def self.run test, command, puts_output = false
+  def self.run test, command, puts_output_on_success = false
     step = new test, command
     step.puts_command
 
-    command = "#{step.command}"
-    unless puts_output and not ARGV.include? "--log"
-      command += " &>#{step.log_file_path}"
-    end
+    command = "#{step.command} &>#{step.log_file_path}"
 
     output = nil
     if command.start_with? 'git '
@@ -86,12 +70,15 @@ class Step
     else
       output = `#{command}`
     end
-    output = IO.read(step.log_file_path) if ARGV.include? "--log"
+    output = IO.read(step.log_file_path)
 
-    step.status = $?.success? ? :passed : :failed
+    success = $?.success?
+    step.status = success ? :passed : :failed
     step.puts_result
-    step.write_html
-    puts output if puts_output and output and not output.empty?
+    if output and output.any? and (not success or puts_output_on_success)
+      puts output
+    end
+    FileUtils.rm step.log_file_path unless ARGV.include? "--keep-logs"
   end
 end
 
@@ -100,12 +87,6 @@ class Test
   attr_reader :core_changed, :formulae
   attr_accessor :steps
 
-  @@css = @@index_html = @@commit_html = nil
-
-  def commit_html_and_css
-    return @@commit_html, @@css
-  end
-
   def initialize argument
     @hash = nil
     @url = nil
@@ -128,57 +109,27 @@ class Test
     @steps = []
     @core_changed = false
     @brewbot_root = Pathname.pwd + "brewbot"
-    if ARGV.include? "--log" or ARGV.include? "--html"
-      FileUtils.mkdir_p @brewbot_root
-    end
-
-    if ARGV.include? "--html" and not @@css
-      require 'erb'
-      root = HOMEBREW_CONTRIBUTED_CMDS/"brew-test-bot"
-      @@css = IO.read root + "brew-test-bot.css"
-      @@index_html = ERB.new IO.read root + "brew-test-bot.index.html.erb"
-      @@commit_html = ERB.new IO.read root + "brew-test-bot.commit.html.erb"
-    end
-  end
-
-  def write_root_html status
-    return unless ARGV.include? "--html"
-
-    FileUtils.mv Dir.glob("*.txt"), @log_root
-    open(@log_root + "status.txt", "w") do |file|
-      file.write status
-    end
-
-    dirs = []
-    dates = []
-    statuses = []
-
-    Pathname.glob("#{@brewbot_root}/*/status.txt").each do |result|
-       dirs << result.dirname.basename
-       status_file = result.dirname + "status.txt"
-       dates << File.mtime(status_file).strftime("%T %D")
-       statuses << IO.read(status_file)
-    end
-
-    open(@brewbot_root + "index.html", "w") do |index|
-      css = @@css
-      index.write @@index_html.result binding
-    end
+    FileUtils.mkdir_p @brewbot_root
   end
 
   def git arguments
     Dir.chdir HOMEBREW_REPOSITORY do
       `git #{arguments}`
+      return $?.success?
     end
   end
 
   def download
     def current_sha1
-      git('rev-parse --short HEAD').strip
+      Dir.chdir HOMEBREW_REPOSITORY do
+        `git rev-parse --short HEAD`.strip
+      end
     end
 
     def current_branch
-      git('symbolic-ref HEAD').gsub('refs/heads/', '').strip
+      Dir.chdir HOMEBREW_REPOSITORY do
+        `git symbolic-ref HEAD`.gsub('refs/heads/', '').strip
+      end
     end
 
     @category = __method__
@@ -208,15 +159,11 @@ class Test
     end
 
     @log_root = @brewbot_root + @name
-    if ARGV.include? "--log" or ARGV.include? "--html"
-      FileUtils.mkdir_p @log_root
-    end
+    FileUtils.mkdir_p @log_root
 
     return unless diff_start_sha1 != diff_end_sha1
     return if @url and steps.last.status != :passed
 
-    write_root_html :running
-
     diff_stat = git "diff #{diff_start_sha1}..#{diff_end_sha1} --name-status"
     diff_stat.each_line do |line|
       status, filename = line.split
@@ -247,6 +194,7 @@ class Test
     dependencies = `brew deps #{formula}`.split("\n")
     dependencies -= `brew list`.split("\n")
     dependencies = dependencies.join(' ')
+    formula_object = Formula.factory(formula)
 
     test "brew audit #{formula}"
     test "brew fetch #{dependencies}" unless dependencies.empty?
@@ -255,7 +203,11 @@ class Test
     test "brew install --verbose --build-bottle #{formula}"
     return unless steps.last.status == :passed
     test "brew bottle #{formula}", true
-    test "brew test #{formula}" if defined? Formula.factory(formula).test
+    bottle_version = bottle_new_version(formula_object)
+    bottle_filename = bottle_filename(formula_object, bottle_version)
+    test "brew uninstall #{formula}"
+    test "brew install #{bottle_filename}"
+    test "brew test #{formula}" if defined? formula_object.test
     test "brew uninstall #{formula}"
     test "brew uninstall #{dependencies}" unless dependencies.empty?
   end
@@ -269,11 +221,11 @@ class Test
   def cleanup_before
     @category = __method__
     return unless ARGV.include? '--cleanup'
-    test 'git stash --include-untracked'
+    git 'stash --all'
     git 'am --abort 2>/dev/null'
     git 'rebase --abort 2>/dev/null'
-    test 'git reset --hard'
-    test 'git clean --force -dx'
+    git 'reset --hard'
+    git 'clean --force -dx'
   end
 
   def cleanup_after
@@ -291,13 +243,15 @@ class Test
 
     if ARGV.include? '--cleanup'
       test 'git reset --hard'
-      test 'git stash pop'
       test 'git gc'
+      git 'stash pop 2>/dev/null'
     end
+
+    FileUtils.rm_rf @brewbot_root unless ARGV.include? "--keep-logs"
   end
 
-  def test cmd, puts_output = false
-    Step.run self, cmd, puts_output
+  def test cmd, puts_output_on_success = false
+    Step.run self, cmd, puts_output_on_success
   end
 
   def check_results
@@ -316,17 +270,7 @@ class Test
         message += "#{step.command}: #{step.status.to_s.upcase}\n"
       end
     end
-
-    write_root_html status
-
-    if ARGV.include? "--comment" and @url
-      username, password = IO.read(File.expand_path('~/.brewbot')).split(':')
-      url = "https://api.github.com/repos/mxcl/homebrew/issues/#{@url}/comments"
-      require 'vendor/multi_json'
-      json = MultiJson.encode(:body => message)
-      curl url, "-X", "POST",  "--user", "#{username}:#{password}", \
-        "--data", json, "-o", "/dev/null"
-    end
+    status == :passed
   end
 
   def self.run argument
@@ -347,9 +291,11 @@ if Pathname.pwd == HOMEBREW_PREFIX and ARGV.include? "--cleanup"
   odie 'cannot use --cleanup from HOMEBREW_PREFIX as it will delete all output.'
 end
 
+any_errors = false
 if ARGV.named.empty?
   # With no arguments just build the most recent commit.
-  Test.run 'HEAD'
+  any_errors = Test.run 'HEAD'
 else
-  ARGV.named.each { |argument| Test.run argument }
+  ARGV.named.each { |argument| any_errors = Test.run(argument) or any_errors }
 end
+exit any_errors ? 0 : 1
diff --git a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.commit.html.erb b/Library/Contributions/cmds/brew-test-bot/brew-test-bot.commit.html.erb
deleted file mode 100644
index c820263a7..000000000
--- a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.commit.html.erb
+++ /dev/null
@@ -1,24 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-	<title>BrewBot: <%= @test.name %></title>
-	<title>BrewBot</title>
-	<style>
-	<%= css %>
-	</style>
-</head>
-<body>
-	<div>
-		<h1><a href="../index.html">BrewBot</a></h1>
-		<h2><%= @test.name %> <%= DateTime.now.strftime "%T %D" %></h2>
-		<table>
-			<% for step in @test.steps %>
-			<tr>
-				<td><span class="prompt">$</span> <%= step.command %></td>
-				<td class='status <%= step.status %>'><a href='<%= step.log_file_path false %> '><%= step.status_upcase %></a></td>
-			</tr>
-			<% end %>
-		</table>
-	</div>
-</body>
-</html>
diff --git a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.css b/Library/Contributions/cmds/brew-test-bot/brew-test-bot.css
deleted file mode 100644
index 3fcefa59a..000000000
--- a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.css
+++ /dev/null
@@ -1,75 +0,0 @@
-body {
-	background: #27221a;
-	color: #f6e6cc;
-	font-family: Helvetica, Arial, sans-serif;
-	font-size: 1.5em;
-}
-
-h1, h2 {
-	text-align: center;
-	font-weight: bold;
-}
-
-h1 {
-	font-size: 3em;
-	padding-top: 0.5em;
-	padding-bottom: 0;
-	margin-bottom: 0;
-	text-shadow: 1px 1px 10px rgba(0,0,0,0.25);
-	color: #D7AF72;
-	letter-spacing: -3px;
-}
-
-h2 {
-	font-size: 0.8em;
-	margin-top: 0;
-	padding-top: 0;
-	padding-bottom: 1em;
-	color: #A1804C;
-}
-
-h1 a:link, h1 a:visited, h2 a:link, h2 a:visited {
-	text-decoration: none;
-}
-
-table {
-	background-color: rgba(0, 0, 0, 0.30);
-	color: white;
-	border-radius: 0.4em;
-	padding-top: 1em;
-	padding-bottom: 1em;
-	font-size: inherit;
-	font-family: monospace;
-	list-style-type: none;
-	margin-left: auto;
-	margin-right: auto;
-}
-
-td {
-	padding-left: 1em;
-	padding-right: 1em;
-}
-
-.prompt {
-	color: #E3D796;
-}
-
-.status {
-	text-align: right;
-}
-
-.running a:link, .running a:visited {
-	color: orange;
-}
-
-.passed a:link, .passed a:visited {
-	color: green;
-}
-
-.failed a:link, .failed a:visited {
-	color: red;
-}
-
-a:active, a:visited {
-	color: inherit;
-}
diff --git a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.index.html.erb b/Library/Contributions/cmds/brew-test-bot/brew-test-bot.index.html.erb
deleted file mode 100644
index d5c5d6913..000000000
--- a/Library/Contributions/cmds/brew-test-bot/brew-test-bot.index.html.erb
+++ /dev/null
@@ -1,23 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-	<title>BrewBot</title>
-	<style>
-	<%= css %>
-	</style>
-</head>
-<body>
-	<div>
-		<h1><a href="index.html">BrewBot</h1>
-		<table>
-			<% dirs.each_with_index do |dir,index| %>
-			<tr>
-				<td><%= dir %></td>
-				<td><%= dates[index] %></td>
-				<td class='status <%= statuses[index] %>'><a href='<%= dir %>/index.html'><%= statuses[index].upcase %></a></td>
-			</tr>
-			<% end  %>
-		</table>
-	</div>
-</body>
-</html>