Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Proposal: brew-commit-test #10671

Closed
wants to merge 1 commit into from

5 participants

@mistydemeo
Owner

I noticed that we've had a few pushes lately which included some syntax or other errors, usually in large batches of edits where it wasn't convenient to individually test every formula.

I put together this command to simplify automated testing of commits before pushing. It goes through every formula that's been changed between the newest commit on the branch you're on and origin/master, and attempts to load and test each of them to pick up errors. A few notes:

  • This only covers formulae. For practical reasons, it wasn't as feasible to automatically test core changes with a generic test. (We should beef up our unit tests for that!)
  • Ruby's lazy evaluation of methods means this won't pick up every problem - for instance, problems in install methods won't be detected. (You'd need to actually run the method for that.) However, everything else should be.
Library/Contributions/examples/brew-commit-test.rb
@@ -0,0 +1,47 @@
+# Automatically tests changed formulae
+
+Dir.chdir HOMEBREW_PREFIX
+branch = `git status`.split("\n").first.match(/On branch /).post_match
+revisions = `git rev-list #{branch} ^origin/master`.chomp!
+if revisions.nil?
+ ohai "No commits to test!"
+ exit
+end
+
+files = revisions.split.collect do |commit|
+ `git show --pretty="format:" --name-only #{commit}`.chomp!.split
+end.flatten!
+
+files.reject! {|file| !(file =~ /Library\/Formula/)}
@mistydemeo Owner

This would look less silly if only 1.8 had #keep_if.

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

Along these lines, I use this shell script as a pre-commit hook:

#!/bin/sh

head=$(git rev-parse --verify -q --short=10 HEAD)

git diff-index --cached --name-only --diff-filter=AM HEAD |
while read file
do
    if echo "$file" | grep -q "Library/Formula/"
    then
        git checkout-index -q -f --prefix="/tmp/$head/" -- "$file"
        brew audit "/tmp/$head/$file"
    fi
done

(It's actually a bit more complex than that, but the other parts aren't really relevant here.)

The script will take any formulae that have staged changes, check them out into a temporary directory and run brew audit on them; since brew audit will exit(1) if there are issues, the commit will fail.

If I really need to commit something that fails audit, I can commit with git commit --no-verify.

Library/Contributions/examples/brew-commit-test.rb
@@ -0,0 +1,47 @@
+# Automatically tests changed formulae
+
+Dir.chdir HOMEBREW_PREFIX
+branch = `git status`.split("\n").first.match(/On branch /).post_match
+revisions = `git rev-list #{branch} ^origin/master`.chomp!
+if revisions.nil?
+ ohai "No commits to test!"
+ exit
+end
+
+files = revisions.split.collect do |commit|
+ `git show --pretty="format:" --name-only #{commit}`.chomp!.split
@jacknagel Owner

I think you can get this with

git diff-tree -r --no-commit-id --name-only $commit

without the cost of invoking the git porcelain.

@mistydemeo Owner

Nice, thanks. That's definitely an improvement. Tweaked that, along with a few other changes (DRYer code, brew audit included).

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

Sounds great! You guys are swell.

Library/Contributions/examples/brew-commit-test.rb
@@ -0,0 +1,44 @@
+# Automatically tests changed formulae
+
+Dir.chdir HOMEBREW_PREFIX
@jacknagel Owner

Probably want HOMEBREW_REPOSITORY here, and below.

@mistydemeo Owner

Good point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Library/Contributions/examples/brew-commit-test.rb
@@ -0,0 +1,44 @@
+# Automatically tests changed formulae
+
+Dir.chdir HOMEBREW_PREFIX
+branch = `git status`.split("\n").first.match(/On branch /).post_match
@jacknagel Owner

You can use git symbolic-ref -q HEAD (will give e.g. "refs/heads/master" which is a valid ref name).

@mistydemeo Owner

Excellent, thanks. Was looking for something along these lines and hadn't found it. Obviously didn't look hard enough!

@jacknagel Owner

Yeah it seems like there should be a more obvious way to do it, but apparently not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@mistydemeo mistydemeo New external command: brew-commit-test
brew-commit-test allows for simple automatic testing of commits before
pushing. It will test all formula files in any commits that haven't been
pushed to master, and print out any exceptions found in them.

Its testing is somewhat simplistic and won't catch all errors,
so use it judiciously - a clean bill of health doesn't mean there are no
bugs!
bc0e31d
@adamv
Owner

Is this closable after the checkin referenced here, or is there more to do?

@mikemcquaid
Owner

FYI brew test-bot does this and the incoming "secret thing" will too. @mistydemeo Can you merge or close? Cheers!

@mistydemeo
Owner

Didn't realize this was still open! Doesn't seem like it's too necessary, closing.

@mistydemeo mistydemeo closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 4, 2012
  1. @mistydemeo

    New external command: brew-commit-test

    mistydemeo authored
    brew-commit-test allows for simple automatic testing of commits before
    pushing. It will test all formula files in any commits that haven't been
    pushed to master, and print out any exceptions found in them.
    
    Its testing is somewhat simplistic and won't catch all errors,
    so use it judiciously - a clean bill of health doesn't mean there are no
    bugs!
This page is out of date. Refresh to see the latest.
Showing with 44 additions and 0 deletions.
  1. +44 −0 Library/Contributions/examples/brew-commit-test.rb
View
44 Library/Contributions/examples/brew-commit-test.rb
@@ -0,0 +1,44 @@
+# Automatically tests changed formulae
+
+Dir.chdir HOMEBREW_REPOSITORY
+branch = `git symbolic-ref -q HEAD`.chomp!
+revisions = `git rev-list #{branch} ^origin/master`.chomp!
+if revisions.nil?
+ ohai "No commits to test!"
+ exit
+end
+
+files = revisions.split.collect do |commit|
+ `git diff-tree -r --no-commit-id --name-only #{commit}`.chomp!.split
+end.flatten!
+
+files.reject! {|file| !(file =~ /Library\/Formula/)}
+opoo "No files were tested." if files.empty?
+
+files.each do |file|
+ exceptions = []
+ source = HOMEBREW_REPOSITORY+file
+ begin; require source.to_s
+ rescue Exception
+ exceptions << "#{Tty.yellow}Loading formula:#{Tty.reset} " + $!
+ end
+ begin; formula = Formula.factory source.stem
+ rescue Exception
+ exceptions << "#{Tty.yellow}Creating formula object:#{Tty.reset} " + $!
+ end
+
+ audit = `brew audit #{source.stem}`.chomp! if exceptions.empty?
+ exceptions << "#{Tty.yellow}Audit failure:#{Tty.reset} " + audit.split("\n")[1..-2].join("\n") if audit
+
+ [:options,:patches,:caveats].each do |option|
+ begin; formula.send(option)
+ rescue Exception
+ exceptions << "#{Tty.yellow}#{option.to_s.capitalize}:#{Tty.reset} " + $!
+ end
+ end
+
+ if !exceptions.empty?
+ onoe "#{file} failed with the following exception(s):"
+ puts exceptions
+ end
+end
Something went wrong with that request. Please try again.