From 8b828960815e681e5b5dbe22ba8de0a0953b1441 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Thu, 10 Jan 2013 18:44:50 -0600 Subject: [PATCH 1/4] Added ability to customize repo names and have slashes in names. --- barkeep_server.rb | 6 +-- lib/admin_routes.rb | 3 +- lib/api.rb | 5 +- lib/api_routes.rb | 21 +++++--- lib/meta_repo.rb | 31 +++++++---- public/coffee/repos.coffee | 16 +++++- public/css/admin.scss | 4 +- test/stub_helper.rb | 6 +-- test/unit/api_test.rb | 90 ++++++++++++++++++-------------- test/unit/barkeep_server_test.rb | 4 +- test/unit/emails_test.rb | 2 +- test/unit/saved_search_test.rb | 2 +- views/admin/repos.erb | 4 +- 13 files changed, 119 insertions(+), 75 deletions(-) diff --git a/barkeep_server.rb b/barkeep_server.rb index d2176e7e..adba065b 100644 --- a/barkeep_server.rb +++ b/barkeep_server.rb @@ -297,10 +297,10 @@ def ensure_required_params(*required_params) end end - get "/commits/:repo_name/:sha" do + get %r{^/commits/(.+)/(.+)$} do MetaRepo.instance.scan_for_new_repos - repo_name = params[:repo_name] - sha = params[:sha] + repo_name = params[:captures][0] + sha = params[:captures][1] halt 404, "No such repository: #{repo_name}" unless GitRepo[:name => repo_name] commit = MetaRepo.instance.db_commit(repo_name, sha) unless commit diff --git a/lib/admin_routes.rb b/lib/admin_routes.rb index 737d2523..0885cdc3 100644 --- a/lib/admin_routes.rb +++ b/lib/admin_routes.rb @@ -91,8 +91,9 @@ class BarkeepServer < Sinatra::Base # - url post "/admin/repos/create_new_repo" do halt 400, "'url' is required." if (params[:url] || "").strip.empty? + halt 400, "'name' is required." if (params[:name] || "").strip.empty? begin - add_repo params[:url] + add_repo params[:url], params[:name] rescue RuntimeError => e halt 400, e.message end diff --git a/lib/api.rb b/lib/api.rb index 4abca9fe..96ca6830 100644 --- a/lib/api.rb +++ b/lib/api.rb @@ -7,12 +7,11 @@ require "resque_jobs/clone_new_repo" module Api - def add_repo(url) + def add_repo(repo_url, repo_name) raise "This is not a valid URL." unless Addressable::URI.parse(url) - repo_name = File.basename(url, ".*") repo_path = File.join(REPOS_ROOT, repo_name) raise "There is already a folder named \"#{repo_name}\" in #{REPOS_ROOT}." if File.exists?(repo_path) - Resque.enqueue(CloneNewRepo, repo_name, url) + Resque.enqueue(CloneNewRepo, repo_name, repo_url) end # Generate a random API key or API secret for a user. diff --git a/lib/api_routes.rb b/lib/api_routes.rb index 1d21ebbf..c78a5586 100644 --- a/lib/api_routes.rb +++ b/lib/api_routes.rb @@ -32,7 +32,9 @@ def api_error(status, message) post "/api/add_repo" do ensure_required_params :url begin - add_repo params[:url] + url = params[:url] + name = params[:name] || File.basename(url, ".*") + add_repo url, name rescue RuntimeError => e api_error 400, e.message end @@ -49,30 +51,33 @@ def api_error(status, message) nil end - get "/api/commits/:repo_name/:sha" do + get %r{^/api/commits/(.+)/(.+)$} do fields = params[:fields] ? params[:fields].split(",") : nil + repo_name = params[:captures][0] + sha = params[:captures][1] begin - commit = Commit.prefix_match params[:repo_name], params[:sha] + commit = Commit.prefix_match repo_name, sha rescue RuntimeError => e api_error 404, e.message end - format_commit_data(commit, params[:repo_name], fields).to_json + format_commit_data(commit, repo_name, fields).to_json end # NOTE(caleb): Large GET requests are rejected by the Ruby web servers we use. (Unicorn, in particular, # doesn't seem to like paths > 1k and rejects them silently.) Hence, to batch-request commit data, we must # use a POST. - post "/api/commits/:repo_name" do + post %r{^/api/commits/(.+)$} do shas = params[:shas].split(",") fields = params[:fields] ? params[:fields].split(",") : nil + repo_name = params[:captures][0] commits = {} shas.each do |sha| begin - commit = Commit.prefix_match params[:repo_name], sha + commit = Commit.prefix_match repo_name, sha rescue RuntimeError => e api_error 404, e.message end - commits[commit.sha] = format_commit_data(commit, params[:repo_name], fields) + commits[commit.sha] = format_commit_data(commit, repo_name, fields) end commits.to_json end @@ -86,7 +91,7 @@ def format_commit_data(commit, repo_name, fields) :approved_by => commit.approved? ? "#{approver.name} <#{approver.email}>" : nil, :approved_at => commit.approved? ? commit.approved_at.to_i : nil, :comment_count => commit.comment_count, - :link => "http://#{BARKEEP_HOSTNAME}/commits/#{params[:repo_name]}/#{commit.sha}" + :link => "http://#{BARKEEP_HOSTNAME}/commits/#{repo_name}/#{commit.sha}" } fields ? commit_data.select { |key, value| fields.include? key.to_s } : commit_data end diff --git a/lib/meta_repo.rb b/lib/meta_repo.rb index d126e4ce..3a6c78a8 100644 --- a/lib/meta_repo.rb +++ b/lib/meta_repo.rb @@ -34,19 +34,33 @@ def initialize(repos_root) # Loads in any new repos from the repos_root. def scan_for_new_repos() load_repos if repos_out_of_date? end + def all_repo_names_recurse(path) + if File.exist?("#{path}/.git") + [Pathname.new(path).relative_path_from(Pathname.new(@repos_root)).to_s] + else + names = [] + repo_paths = Dir.glob("#{path}/*/") + repo_paths.each do |sub_path| + names += all_repo_names_recurse(sub_path) + end + names + end + end + + def all_repo_names + all_repo_names_recurse(@repos_root) + end + def load_repos @repos = [] @repo_names_and_ids_to_repos = {} @repo_name_to_id = {} - repo_paths = Dir.glob("#{@repos_root}/*/") - - repo_paths.each do |path| - path = Pathname.new(path).realpath.to_s # Canonical path - name = File.basename(path) + all_repo_names.each do |name| + path = Pathname.new("#{@repos_root}/#{name}").realpath.to_s # Canonical path id = GitRepo.find_or_create(:name => name, :path => path).id grit_repo = create_grit_repo_for_name(name) - next unless grit_repo && grit_repo.has_refs? + return unless grit_repo && grit_repo.has_refs? @repos << grit_repo @repo_name_to_id[name] = id @repo_names_and_ids_to_repos[name] = grit_repo @@ -70,7 +84,7 @@ def grit_commit(repo_name_or_id, sha) grit_commit = grit_repo.commit(sha) return nil unless grit_commit - grit_commit.repo_name = File.basename(grit_repo.working_dir) + grit_commit.repo_name = Pathname.new(grit_repo.working_dir).relative_path_from(Pathname.new(@repos_root)).to_s grit_commit end @@ -353,8 +367,7 @@ def self.git_command_options(search_options) private def repos_out_of_date? - repo_names = Dir.glob("#{@repos_root}/*/").map { |path| File.basename(path) } - Set.new(repo_names) != Set.new(@repos.map(&:name)) + Set.new(all_repo_names) != Set.new(@repos.map(&:name)) end # Creates a new Grit::Repo object for the given path. diff --git a/public/coffee/repos.coffee b/public/coffee/repos.coffee index bbd71ec5..1fdd97fe 100644 --- a/public/coffee/repos.coffee +++ b/public/coffee/repos.coffee @@ -2,14 +2,26 @@ window.Repos = init: -> $("button#clone").click => repoUrl = $("#newRepoUrl").val() + repoName = $("#newRepoName").val() $.ajax type: "post" url: "/admin/repos/create_new_repo" - data: { url: repoUrl } + data: { url: repoUrl, name: repoName } dataType: "json" - success: => @showConfirmationMessage("#{repoUrl} has been scheduled to be cloned.") + success: => @showConfirmationMessage("#{repoUrl} has been scheduled to be cloned as #{repoName}.") error: (response) => @showConfirmationMessage(response.responseText) + $("#newRepoUrl").bind "propertychange keyup input paste", => + repoUrl = $("#newRepoUrl").val() + match = /.*(?:\/|:)([^/:]+)\/*$/.exec repoUrl + if match + name = match[1] + name = (/\s*([^\s]+)/.exec name)[1] + suffix_match = /(.*)\.git$/.exec name + if suffix_match + name = suffix_match[1] + $("#newRepoName").val(name) + $(".trash").click (e) => repoRow = $(e.target).closest("tr") repoName = repoRow.find("td:nth-of-type(1)").html() diff --git a/public/css/admin.scss b/public/css/admin.scss index d33159e6..ad648ad8 100644 --- a/public/css/admin.scss +++ b/public/css/admin.scss @@ -64,10 +64,12 @@ #cloneForm { label { font-weight: bold; } input { - width: 450px; font-size: 18px; margin: 8px 0; } + #newRepoUrl { + width: 450px; + } button { padding: 4px 6px; font-size: 14px; diff --git a/test/stub_helper.rb b/test/stub_helper.rb index 8a1ddac2..f2c5f092 100644 --- a/test/stub_helper.rb +++ b/test/stub_helper.rb @@ -4,16 +4,16 @@ module StubHelper # Creates a Commit. # - user: a User who is the author of this commit. - def stub_commit(sha, user) + def stub_commit(repo_name, sha, user) commit = Commit.new(:sha => sha) - stub(commit).git_repo { GitRepo.new(:name => "my_repo") } + stub(commit).git_repo { GitRepo.new(:name => repo_name) } stub(commit).user { user } commit_author = user.name.dup stub(commit_author).user { user } grit_commit = OpenStruct.new( :id => sha, :sha => sha, :id_abbrev => sha, - :repo_name => "my_repo", + :repo_name => repo_name, :short_message => "message", :author => Grit::Actor.new(user.name, user.email), :date => Time.now, :diffs => []) stub(commit).grit_commit { grit_commit } diff --git a/test/unit/api_test.rb b/test/unit/api_test.rb index 0a21ef49..1548106b 100644 --- a/test/unit/api_test.rb +++ b/test/unit/api_test.rb @@ -17,62 +17,72 @@ def app() BarkeepServer.new end end context "get commit" do - def approved_stub_commit(sha) - commit = stub_commit(sha, @user) + def approved_stub_commit(repo_name, sha) + commit = stub_commit(repo_name, sha, @user) stub_many commit, :approved_by_user_id => 42, :approved_by_user => @user, :comment_count => 155 commit end should "return a 404 and human-readable error message when given a bad repo or sha" do - stub(@@repo).db_commit("my_repo", "sha1") { nil } # No results - get "/api/commits/my_repo/sha1" - assert_status 404 - assert JSON.parse(last_response.body).include? "error" + ["my_repo", "namespace/my_repo"].each do |repo_name| + stub(@@repo).db_commit(repo_name, "sha1") { nil } # No results + get "/api/commits/#{repo_name}/sha1" + assert_status 404 + assert JSON.parse(last_response.body).include? "error" + end end should "return the relevant metadata for an unapproved commit as expected" do - unapproved_commit = stub_commit("sha1", @user) - stub_many unapproved_commit, :approved_by_user_id => nil, :comment_count => 0 - stub(Commit).prefix_match("my_repo", "sha1") { unapproved_commit } - get "/api/commits/my_repo/sha1" - assert_status 200 - result = JSON.parse(last_response.body) - refute result["approved"] - assert_equal 0, result["comment_count"] - assert_match %r[commits/my_repo/sha1$], result["link"] + ["my_repo", "namespace/my_repo"].each do |repo_name| + unapproved_commit = stub_commit(repo_name, "sha1", @user) + stub_many unapproved_commit, :approved_by_user_id => nil, :comment_count => 0 + stub(Commit).prefix_match(repo_name, "sha1") { unapproved_commit } + get "/api/commits/#{repo_name}/sha1" + assert_status 200 + result = JSON.parse(last_response.body) + refute result["approved"] + assert_equal 0, result["comment_count"] + assert_match %r[commits/#{repo_name}/sha1$], result["link"] + end end should "return the relevant metadata for an approved commit as expected" do - approved_commit = approved_stub_commit("sha1") - stub(Commit).prefix_match("my_repo", "sha1") { approved_commit } - get "/api/commits/my_repo/sha1" - assert_status 200 - result = JSON.parse(last_response.body) - assert result["approved"] - assert_equal 155, result["comment_count"] - assert_equal "The Barkeep ", result["approved_by"] + ["my_repo", "namespace/my_repo"].each do |repo_name| + approved_commit = approved_stub_commit(repo_name, "sha1") + stub(Commit).prefix_match(repo_name, "sha1") { approved_commit } + get "/api/commits/#{repo_name}/sha1" + assert_status 200 + result = JSON.parse(last_response.body) + assert result["approved"] + assert_equal 155, result["comment_count"] + assert_equal "The Barkeep ", result["approved_by"] + end end should "allow for fetching multiple shas at once using the post route" do - commit1 = approved_stub_commit("sha1") - commit2 = approved_stub_commit("sha2") - stub(Commit).prefix_match("my_repo", "sha1") { commit1 } - stub(Commit).prefix_match("my_repo", "sha2") { commit2 } - post "/api/commits/my_repo", :shas => "sha1,sha2" - assert_status 200 - result = JSON.parse(last_response.body) - assert_equal 2, result.size - ["sha1", "sha2"].each { |sha| assert_equal 155, result[sha]["comment_count"] } + ["my_repo", "namespace/my_repo"].each do |repo_name| + commit1 = approved_stub_commit(repo_name, "sha1") + commit2 = approved_stub_commit(repo_name, "sha2") + stub(Commit).prefix_match(repo_name, "sha1") { commit1 } + stub(Commit).prefix_match(repo_name, "sha2") { commit2 } + post "/api/commits/#{repo_name}", :shas => "sha1,sha2" + assert_status 200 + result = JSON.parse(last_response.body) + assert_equal 2, result.size + ["sha1", "sha2"].each { |sha| assert_equal 155, result[sha]["comment_count"] } + end end should "only return requested fields" do - approved_commit = approved_stub_commit("sha1") - stub(Commit).prefix_match("my_repo", "sha1") { approved_commit } - get "/api/commits/my_repo/sha1?fields=approved" - assert_status 200 - result = JSON.parse(last_response.body) - assert result["approved"] - assert_equal 1, result.size + ["my_repo", "namespace/my_repo"].each do |repo_name| + approved_commit = approved_stub_commit(repo_name, "sha1") + stub(Commit).prefix_match(repo_name, "sha1") { approved_commit } + get "/api/commits/#{repo_name}/sha1?fields=approved" + assert_status 200 + result = JSON.parse(last_response.body) + assert result["approved"] + assert_equal 1, result.size + end end end @@ -97,7 +107,7 @@ def get_with_signature(url, params) @whitelist_routes = BarkeepServer::AUTHENTICATION_WHITELIST_ROUTES.dup @whitelist_routes.each { |route| BarkeepServer::AUTHENTICATION_WHITELIST_ROUTES.delete route } - approved_commit = stub_commit("sha1", @user) + approved_commit = stub_commit("my_repo", "sha1", @user) stub_many approved_commit, :approved_by_user_id => 42, :approved_by_user => @user, :comment_count => 155 stub(Commit).prefix_match("my_repo", "sha1") { approved_commit } diff --git a/test/unit/barkeep_server_test.rb b/test/unit/barkeep_server_test.rb index 4d81a02f..a70a5731 100644 --- a/test/unit/barkeep_server_test.rb +++ b/test/unit/barkeep_server_test.rb @@ -19,7 +19,7 @@ def app() BarkeepServer.new end @comment = Comment.new(:text => "howdy ho", :created_at => Time.now) stub(@comment).user { @user } stub(@comment).filter_text { "fancified" } - @commit = stub_commit("commit_id", @user) + @commit = stub_commit("my_repo", "commit_id", @user) stub(@@repo).db_commit { @commit } end @@ -52,7 +52,7 @@ def setup_repo(name) # There are two repos in our system. @repo1 = setup_repo("repo1") @repo2 = setup_repo("repo2") - @commit = stub_commit("sha_123", @user) + @commit = stub_commit("my_repo", "sha_123", @user) stub(@@repo).repos { [@repo1, @repo2] } end diff --git a/test/unit/emails_test.rb b/test/unit/emails_test.rb index ba15efed..dca57668 100644 --- a/test/unit/emails_test.rb +++ b/test/unit/emails_test.rb @@ -9,7 +9,7 @@ class EmailsTest < Scope::TestCase setup do @user = User.new(:name => "jimbo") - @commit = stub_commit("commit_id", @user) + @commit = stub_commit("my_repo", "commit_id", @user) end context "strip_unchanged_blank_lines" do diff --git a/test/unit/saved_search_test.rb b/test/unit/saved_search_test.rb index 5870c284..408def6a 100644 --- a/test/unit/saved_search_test.rb +++ b/test/unit/saved_search_test.rb @@ -24,7 +24,7 @@ class SavedSearchTest < Scope::TestCase context "commits" do setup do @user = User.new(:name => "jimbo") - @commit = stub_commit("commit_id", @user) + @commit = stub_commit("my_repo", "commit_id", @user) @saved_search = SavedSearch.new @git_repo = GitRepo.new @dataset = DatasetStub.new diff --git a/views/admin/repos.erb b/views/admin/repos.erb index 172478be..eb37fe50 100644 --- a/views/admin/repos.erb +++ b/views/admin/repos.erb @@ -14,8 +14,10 @@

Add new repo

-
+ + +
e.g. http://github.com/ooyala/barkeep.git

This repo will be cloned into <%= REPOS_ROOT %>

From 6b6774de952b93e3f88b6d29bb5ceaa8a1a48664 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Fri, 11 Jan 2013 16:07:59 -0600 Subject: [PATCH 2/4] Added second integration test repo for compound names, modified unit tests to take advantage of it. --- .gitmodules | 3 ++ lib/meta_repo.rb | 2 +- test/fixtures/base/test_git_repo | 1 + test/unit/api_test.rb | 11 ++-- test/unit/meta_repo_test.rb | 87 +++++++++++++++++++------------- 5 files changed, 63 insertions(+), 41 deletions(-) create mode 160000 test/fixtures/base/test_git_repo diff --git a/.gitmodules b/.gitmodules index ac46798c..1b02c7be 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "test/fixtures/test_git_repo"] path = test/fixtures/test_git_repo url = git://github.com/ooyala/barkeep_integration_tests.git +[submodule "test/fixtures/base/test_git_repo"] + path = test/fixtures/base/test_git_repo + url = git@github.com:michaelstorm/barkeep_compound_name_integration_tests.git diff --git a/lib/meta_repo.rb b/lib/meta_repo.rb index 3a6c78a8..90c58fea 100644 --- a/lib/meta_repo.rb +++ b/lib/meta_repo.rb @@ -26,7 +26,7 @@ def configure(logger, repos_root) attr_reader :repos def initialize(repos_root) - @repos_root = repos_root + @repos_root = Pathname.new(repos_root).realdirpath.to_s Thread.abort_on_exception = true load_repos end diff --git a/test/fixtures/base/test_git_repo b/test/fixtures/base/test_git_repo new file mode 160000 index 00000000..88e5eef7 --- /dev/null +++ b/test/fixtures/base/test_git_repo @@ -0,0 +1 @@ +Subproject commit 88e5eef73bc508ff297937e7c4b0cecce01ca4d8 diff --git a/test/unit/api_test.rb b/test/unit/api_test.rb index 1548106b..545d4fa8 100644 --- a/test/unit/api_test.rb +++ b/test/unit/api_test.rb @@ -14,6 +14,7 @@ def app() BarkeepServer.new end stub(MetaRepo).instance { @@repo } @user = User.new(:email => "thebarkeep@barkeep.com", :name => "The Barkeep") any_instance_of(BarkeepServer, :current_user => @user) + @repo_names = ["my_repo", "base/my_repo"] end context "get commit" do @@ -24,7 +25,7 @@ def approved_stub_commit(repo_name, sha) end should "return a 404 and human-readable error message when given a bad repo or sha" do - ["my_repo", "namespace/my_repo"].each do |repo_name| + @repo_names.each do |repo_name| stub(@@repo).db_commit(repo_name, "sha1") { nil } # No results get "/api/commits/#{repo_name}/sha1" assert_status 404 @@ -33,7 +34,7 @@ def approved_stub_commit(repo_name, sha) end should "return the relevant metadata for an unapproved commit as expected" do - ["my_repo", "namespace/my_repo"].each do |repo_name| + @repo_names.each do |repo_name| unapproved_commit = stub_commit(repo_name, "sha1", @user) stub_many unapproved_commit, :approved_by_user_id => nil, :comment_count => 0 stub(Commit).prefix_match(repo_name, "sha1") { unapproved_commit } @@ -47,7 +48,7 @@ def approved_stub_commit(repo_name, sha) end should "return the relevant metadata for an approved commit as expected" do - ["my_repo", "namespace/my_repo"].each do |repo_name| + @repo_names.each do |repo_name| approved_commit = approved_stub_commit(repo_name, "sha1") stub(Commit).prefix_match(repo_name, "sha1") { approved_commit } get "/api/commits/#{repo_name}/sha1" @@ -60,7 +61,7 @@ def approved_stub_commit(repo_name, sha) end should "allow for fetching multiple shas at once using the post route" do - ["my_repo", "namespace/my_repo"].each do |repo_name| + @repo_names.each do |repo_name| commit1 = approved_stub_commit(repo_name, "sha1") commit2 = approved_stub_commit(repo_name, "sha2") stub(Commit).prefix_match(repo_name, "sha1") { commit1 } @@ -74,7 +75,7 @@ def approved_stub_commit(repo_name, sha) end should "only return requested fields" do - ["my_repo", "namespace/my_repo"].each do |repo_name| + @repo_names.each do |repo_name| approved_commit = approved_stub_commit(repo_name, "sha1") stub(Commit).prefix_match(repo_name, "sha1") { approved_commit } get "/api/commits/#{repo_name}/sha1?fields=approved" diff --git a/test/unit/meta_repo_test.rb b/test/unit/meta_repo_test.rb index 0bfe140b..efd82751 100644 --- a/test/unit/meta_repo_test.rb +++ b/test/unit/meta_repo_test.rb @@ -15,6 +15,12 @@ class MetaRepoTest < Scope::TestCase @third_commit_on_master = "9f9c5d8" @repo_name = "test_git_repo" + @compound_repo_name = "base/test_git_repo" + + @repo_results = [{ :name => @repo_name, :first_commit => "65a0045", :second_commit => "17de311", + :third_master_commit => "9f9c5d8", :first_cheese_commit => "4a7d3e5"}, + { :name => @compound_repo_name, :first_commit => "cdf4874", :second_commit => "6282db5", + :third_master_commit => "30bcea6", :first_cheese_commit => "c7fb18d"}] end setup_once do @@ -28,7 +34,7 @@ class MetaRepoTest < Scope::TestCase @@repo = MetaRepo.instance # Access the private git repo inside MetaRepo. - @@grit_repo = @@repo.send(:repos)[0] + @@grit_repo = @@repo.get_grit_repo("test_git_repo") end context "grit_commit" do @@ -103,31 +109,36 @@ class MetaRepoTest < Scope::TestCase should "find commits matching an author" do assert_equal [], @@repo.find_commits(@options.merge(:authors => ["nonexistant author"]))[:commits] - result = @@repo.find_commits(@options.merge(:authors => ["Phil Crosby"])) - assert_equal 2, result[:commits].size - assert_equal ["Phil Crosby"], result[:commits].map { |commit| commit.author.name }.uniq + @repo_results.each do |repo_result| + result = @@repo.find_commits(@options.merge(:repos => [repo_result[:name]], :authors => ["Phil Crosby"])) + assert_equal 2, result[:commits].size + assert_equal ["Phil Crosby"], result[:commits].map { |commit| commit.author.name }.uniq - # TODO(philc): the test below should work, but it's 1, not 2. Fix that bug. - # assert_equal 2, results[:count] + # TODO(philc): the test below should work, but it's 1, not 2. Fix that bug. + # assert_equal 2, results[:count] + end end should "find commits matching a branch" do assert_equal [], @@repo.find_commits(@options.merge(:branches => ["nonexistant_branch"]))[:commits] - first_commit_on_cheese_branch = "4a7d3e5" - - result = @@repo.find_commits(@options.merge(:branches => ["cheese"])) - assert_equal [first_commit_on_cheese_branch, @first_commit], result[:commits].map(&:id_abbrev) + @repo_results.each do |repo_result| + result = @@repo.find_commits(@options.merge(:branches => ["cheese"], :repos => [repo_result[:name]])) + assert_equal [repo_result[:first_cheese_commit], repo_result[:first_commit]], result[:commits].map(&:id_abbrev) + end end should "filter out commits olrder than min_commit_date" do - min_commit_date = @@grit_repo.commit(@second_commit).date + min_commit_date = @@repo.get_grit_repo("test_git_repo").commit(@second_commit).date options = @options.merge(:after => min_commit_date, :branches => ["master"], :limit => 100) - commit_ids = @@repo.find_commits(options)[:commits].map(&:id_abbrev) - # The first commit in the repo should be omitted, because it'so lder than min_commit_date. - expected = [@third_commit_on_master, @second_commit].sort - assert_equal expected, (commit_ids & (expected + [@first_commit])).sort + @repo_results.each do |repo_result| + commit_ids = @@repo.find_commits(options.merge(:repos => [repo_result[:name]]))[:commits].map(&:id_abbrev) + + # The first commit in the repo should be omitted, because it'so lder than min_commit_date. + expected = [repo_result[:third_master_commit], repo_result[:second_commit]].sort + assert_equal expected, (commit_ids & (expected + [repo_result[:first_comit]])).sort + end end context "commits_from_repo" do @@ -136,30 +147,36 @@ class MetaRepoTest < Scope::TestCase end should "use a commit_filter_proc to filter out commits from the list of results" do - # This search should include the first_commit and second_commit. - commit_ids = @@repo.commits_from_repo(@@grit_repo, @git_options, 100, :first).map(&:id_abbrev) - assert commit_ids.include?(@first_commit) - assert commit_ids.include?(@second_commit) - - # This search uses a filter_proc to eliminate all commits but the first one. - commit_filter_proc = proc { |commits| commits.select { |commit| commit.id_abbrev == @first_commit } } - commit_ids = @@repo.commits_from_repo(@@grit_repo, @git_options, 2, :first, commit_filter_proc). - map(&:id_abbrev) - assert_equal [@first_commit], commit_ids + @repo_results.each do |repo_result| + grit_repo = @@repo.get_grit_repo(repo_result[:name]) + + # This search should include the first_commit and second_commit. + commit_ids = @@repo.commits_from_repo(grit_repo, @git_options, 100, :first).map(&:id_abbrev) + assert commit_ids.include?(repo_result[:first_commit]) + assert commit_ids.include?(repo_result[:second_commit]) + + # This search uses a filter_proc to eliminate all commits but the first one. + commit_filter_proc = proc { |commits| commits.select { |commit| commit.id_abbrev == repo_result[:first_commit] } } + commit_ids = @@repo.commits_from_repo(grit_repo, @git_options, 2, :first, commit_filter_proc). + map(&:id_abbrev) + assert_equal [repo_result[:first_commit]], commit_ids + end end should "page through commits and pass each page to commit_filter_proc" do - commits_being_filtered = [] - commit_filter_proc = Proc.new do |commits| - commits_being_filtered.push(commits.map(&:id_abbrev)) - commits.select { |commit| commit.id_abbrev == @first_commit } + @repo_results.each do |repo_result| + commits_being_filtered = [] + commit_filter_proc = Proc.new do |commits| + commits_being_filtered.push(commits.map(&:id_abbrev)) + commits.select { |commit| commit.id_abbrev == repo_result[:first_commit] } + end + commit_ids = @@repo.commits_from_repo(@@repo.get_grit_repo(repo_result[:name]), @git_options, 1, :first, commit_filter_proc). + map(&:id_abbrev) + + # commits_from_repo() pages through commits in pages of 2*limit at a time. + assert_equal [[repo_result[:third_master_commit], repo_result[:second_commit]], [repo_result[:first_commit]]], commits_being_filtered + assert_equal [repo_result[:first_commit]], commit_ids end - commit_ids = @@repo.commits_from_repo(@@grit_repo, @git_options, 1, :first, commit_filter_proc). - map(&:id_abbrev) - - # commits_from_repo() pages through commits in pages of 2*limit at a time. - assert_equal [[@third_commit_on_master, @second_commit], [@first_commit]], commits_being_filtered - assert_equal [@first_commit], commit_ids end end end From 551fc3f14b991803fa52b64f868aa23eebb894e8 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Fri, 11 Jan 2013 19:04:44 -0600 Subject: [PATCH 3/4] Updated repo cloning integration test to include compound paths. --- .../clone_new_repo_integration_test.rb | 36 ++++++++++--------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/test/integration/clone_new_repo_integration_test.rb b/test/integration/clone_new_repo_integration_test.rb index 1910c5d7..58ce10fe 100644 --- a/test/integration/clone_new_repo_integration_test.rb +++ b/test/integration/clone_new_repo_integration_test.rb @@ -10,38 +10,42 @@ class CloneNewRepoIntegrationTest < Scope::TestCase include IntegrationTestHelper setup_once do - @@test_repo_name = "clone_repo_integration_test" + @@test_repo_names = ["clone_repo_integration_test", "base/clone_repo_integration_test"] + @@bad_test_repo_names = ["bad_repo_integration_test", "bad_base/bad_repo_integration_test"] # We could also clone this repo from git@github.com:ooyala/barkeep_integration_tests.git, but that takes # about 5 seconds. Cloning from the local disk is much faster. @@test_repo_url = File.expand_path(File.join(File.dirname(__FILE__), "../fixtures/test_git_repo")) - delete_repo(@@test_repo_name) + @@test_repo_names.each { |repo_name| delete_repo(repo_name) } end teardown_once do - delete_repo(@@test_repo_name) + @@test_repo_names.each { |repo_name| delete_repo(repo_name) } end should "clone a new repo to the correct place on disk" do - assert_equal false, File.exists?(repo_path(@@test_repo_name)) - CloneNewRepo.perform(@@test_repo_name, @@test_repo_url) - assert File.exists?(repo_path(@@test_repo_name)), "repo should've been cloned, but it's not on disk." - - # Ensure it successfully completed a full clone (i.e. commits can now be read by Grit). - first_commit = "65a0045e7ac5329d76e6644aa2fb427b78100a7b" - grit = Grit::Repo.new(repo_path(@@test_repo_name)) - assert_equal 1, grit.commits(first_commit).size + @@test_repo_names.each do |repo_name| + assert_equal false, File.exists?(repo_path(repo_name)) + CloneNewRepo.perform(repo_name, @@test_repo_url) + assert File.exists?(repo_path(repo_name)), "repo should've been cloned, but it's not on disk." + + # Ensure it successfully completed a full clone (i.e. commits can now be read by Grit). + first_commit = "65a0045e7ac5329d76e6644aa2fb427b78100a7b" + grit = Grit::Repo.new(repo_path(repo_name)) + assert_equal 1, grit.commits(first_commit).size + end end should "if a repo cannot be cloned, no trace of it should be left on disk" do - repo_name = "bad_repo_integration_test" - FileUtils.rm_rf(repo_path(repo_name)) - CloneNewRepo.perform(repo_name, "bad_repo/url") - assert_equal false, File.exists?(repo_path(repo_name)) + @@bad_test_repo_names.each do |repo_name| + FileUtils.rm_rf(repo_path(repo_name)) + CloneNewRepo.perform(repo_name, "bad_repo/url") + assert_equal false, File.exists?(repo_path(repo_name)) + end end - def repo_path(repo_name) File.join(REPOS_ROOT, @@test_repo_name) end + def repo_path(repo_name) File.join(REPOS_ROOT, repo_name) end def delete_repo(repo_name) FileUtils.rm_rf(repo_path(repo_name)) if File.exists?(repo_path(repo_name)) end From e8246abd1f78d7ac72f85a1c452e25273bef3386 Mon Sep 17 00:00:00 2001 From: Michael Storm Date: Sat, 12 Jan 2013 15:54:37 -0600 Subject: [PATCH 4/4] Allow a compound repo name to be specified as an argument to the 'barkeep client' command. --- client/lib/barkeep/commit.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/lib/barkeep/commit.rb b/client/lib/barkeep/commit.rb index add5f099..26743bdb 100644 --- a/client/lib/barkeep/commit.rb +++ b/client/lib/barkeep/commit.rb @@ -40,8 +40,8 @@ def self.commit(configuration) def self.parse_commit(commit_specification) case commit_specification - when %r{^[^/]+/#{SHA_REGEX}$} # foo/abc123 - commit_specification.split "/" + when %r{^(\S+)/(#{SHA_REGEX})$} # foo/abc123 + [$1, $2] when /^#{SHA_REGEX}$/ # abc123 repo = File.basename(`git rev-parse --show-toplevel`).strip if repo.empty?