Merge pull request #7 from frankshearar/make-label-colours-conform-to…

…-master

Ensure that labels' colours match those of the master repository

Gemfile

@@ -1,9 +1,9 @@
 source 'http://rubygems.org'
+gem 'hashie'
 gem 'highline'
 gem 'json', '~> 1.7.7'
 gem 'octokit'
 gem 'rake'
-gem 'hamsterdam'
 gem 'safe_yaml'
 gem 'trollop'
 

Gemfile.lock

@@ -26,9 +26,6 @@ GEM
     guard-rspec (3.0.1)
       guard (>= 1.8)
       rspec (~> 2.13)
-    hamster (0.4.3)
-    hamsterdam (1.0.8)
-      hamster
     hashie (2.0.5)
     highline (1.6.19)
     json (1.7.7)
@@ -84,7 +81,7 @@ PLATFORMS
 DEPENDENCIES
   coveralls
   guard-rspec
-  hamsterdam
+  hashie
   highline
   json (~> 1.7.7)
   octokit

lib/octoherder/configuration.rb

@@ -1,4 +1,3 @@
-require 'hamsterdam'
 require 'octokit.rb'
 require 'time'
 require 'safe_yaml'
@@ -8,7 +7,7 @@
 module OctoHerder
   NEUTRAL_TONE = 'cccccc'
 
-  Configuration = Hamsterdam::Struct.define(:master, :repositories, :milestones, :columns, :labels)
+  Configuration = Struct.new(:master, :repositories, :milestones, :columns, :labels)
   class Configuration
     def self.read_file path
       File.open(path.to_s, "r") { |f| self.read_string f.read }
@@ -23,10 +22,10 @@ def self.read_string source
 
       master = data.fetch('master')
       columns = data.fetch('columns', [])
-      labels = data.fetch('labels', [])
+      labels = data.fetch('labels', []).map(&:to_s)
       milestones = data.fetch('milestones', [])
       repositories = data.fetch('repositories', [])
-      Configuration.new master: master, repositories: repositories, milestones: milestones, columns: columns, labels: labels
+      Configuration.new master, repositories, milestones, columns, labels
     end
 
     def self.generate_configuration octokit_connection, master_repo_name
@@ -45,7 +44,7 @@ def self.generate_configuration octokit_connection, master_repo_name
         }
       }
 
-      Configuration.new master: master_repo_name, repositories: repositories, milestones: milestones, columns: columns, labels: labels
+      Configuration.new master_repo_name, repositories, milestones, columns, labels
     end
 
     def write_file path
@@ -60,8 +59,8 @@ def write_file path
       }
     end
 
-    # Ensure that every repository has the specified labels. Labels always
-    # have the same, neutral, colour.
+    # Ensure that every repository has the specified labels. Labels' colours
+    # match those of the primary repository.
     def update_labels octokit_connection
       ([master] + repositories).map { |str|
         Octokit::Repository.new str
@@ -71,17 +70,25 @@ def update_labels octokit_connection
     end
 
     def update_link_labels octokit_connection
-      actual_labels = octokit_connection.labels(Octokit::Repository.new(master)).map(&:name)
       link_labels = repositories.map { | str | "Link <=> #{str}" }
-      add_new_labels octokit_connection, master, link_labels
+      add_new_labels octokit_connection, Octokit::Repository.new(master), link_labels
     end
 
     def add_new_labels octokit_connection, repository, labels
-      existing_labels = octokit_connection.labels(repository).map(&:name)
+      master_labels = Hash[octokit_connection.labels(Octokit::Repository.new(master)).map {|l| [l.name, l.color]}]
 
-      (labels - existing_labels).each { | label |
+      existing_labels = octokit_connection.labels(repository).map {|l| [l.name, l.color]}
+      existing_labels.each { |name, colour|
+        target_colour = master_labels.fetch(name, NEUTRAL_TONE)
+        if colour != target_colour then
+          octokit_connection.update_label(repository, name, {color: target_colour})
+        end
+      }
+
+      existing_label_names = existing_labels.map(&:first)
+      (labels - existing_label_names).each { | label |
         begin
-          octokit_connection.add_label(repository, label, NEUTRAL_TONE)
+          octokit_connection.add_label(repository, label, {color: master_labels.fetch(label, NEUTRAL_TONE)})
         rescue Octokit::Error => e
           # Referencing an instvar is disgusting (and fragile). But how else do
           # we get this very useful debugging info? The response body isn't

spec/cli_spec.rb

@@ -1,8 +1,6 @@
 require 'spec_helper'
 require 'octoherder'
-require 'rspec'
 require 'tempfile'
-require 'data/sample-github-responses'
 
 module OctoHerder
   describe CLI do

spec/configuration_spec.rb

@@ -1,8 +1,6 @@
 require 'spec_helper'
-require 'rspec'
 require 'octoherder/configuration'
-require 'data/sample-github-responses'
-require 'ostruct'
+require 'hashie'
 
 module OctoHerder
   describe Configuration do
@@ -33,7 +31,7 @@ module OctoHerder
       let(:conf) { Configuration.read_file conf_file }
       let(:source) { YAML.load_file conf_file }
       let (:master) { source.fetch('master') }
-      let (:labels) { source.fetch('labels') }
+      let (:labels) { source.fetch('labels').map(&:to_s) }
       let (:columns) { source.fetch('columns') }
       let (:linked_repos) { source.fetch('repositories') }
       let (:milestones) { source.fetch('milestones') }
@@ -59,27 +57,10 @@ module OctoHerder
         expect(conf.milestones.count).to equal(source['milestones'].count)
       end
 
-      it "should add labels to repositories that lack some" do
-        connection.stub(:labels).and_return([], [], [])
-        connection.stub(:add_label)
-        connection.should_receive(:labels).exactly(repo_count).times
-        labels.each { |label|
-          connection.should_receive(:add_label).with(an_instance_of(Octokit::Repository), label, OctoHerder::NEUTRAL_TONE)
-        }
-
-        conf.update_labels connection
+      it "should treat all labels as Strings" do
+        expect(conf.labels).to eq(conf.labels.map(&:to_s))
       end
 
-      it "should add columns to repositories that lack some" do
-        connection.stub(:labels).and_return([], [], [])
-        connection.stub(:add_label)
-        connection.should_receive(:labels).exactly(repo_count).times
-        columns.each { |label|
-          connection.should_receive(:add_label).with(an_instance_of(Octokit::Repository), label, OctoHerder::NEUTRAL_TONE)
-        }
-
-        conf.update_labels connection
-      end
 
       it "should ask all repositories for their milestones" do
         connection.stub(:list_milestones).and_return(LIST_MILESTONES_FOR_A_REPOSITORY,
@@ -93,7 +74,16 @@ module OctoHerder
       it "should add any missing huboard repository links" do
         connection.stub(:list_milestones).and_return(LIST_MILESTONES_FOR_A_REPOSITORY)
         connection.stub(:create_milestone)
-        connection.stub(:labels).and_return([OpenStruct.new(name: "Link <=> " + conf.repositories.first)])
+
+        connection.stub(:labels) { |repo|
+          repo_name = "#{repo.username}/#{repo.name}"
+          if repo_name == master then
+            [{name: "Link <=> " + conf.repositories.first}].map {|h| Hashie::Mash.new h}
+          else
+            []
+          end
+        }
+
         connection.stub(:add_label)
         # This happens to also check that we don't add any link labels to the
         # linked repositories.
@@ -106,9 +96,9 @@ module OctoHerder
         connection.stub(:list_milestones).and_return(LIST_MILESTONES_FOR_A_REPOSITORY)
         connection.stub(:create_milestone)
 
-          connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-1', {'state' => 'closed'}).exactly(repo_count).times
-          connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-2', {'due_on' => Time.iso8601('2011-04-10T20:09:31Z')}).exactly(repo_count).times
-          connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-3', {'state' => 'open', 'description' => 'The third step in total world domination.'}).exactly(repo_count).times
+        connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-1', {'state' => 'closed'}).exactly(repo_count).times
+        connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-2', {'due_on' => Time.iso8601('2011-04-10T20:09:31Z')}).exactly(repo_count).times
+        connection.should_receive(:create_milestone).with(an_instance_of(Octokit::Repository), 'milestone-3', {'state' => 'open', 'description' => 'The third step in total world domination.'}).exactly(repo_count).times
 
         conf.update_milestones connection
       end
@@ -159,7 +149,7 @@ module OctoHerder
            "url" => "https =>//api.github.com/repos/me/mine/labels/critical",
            "name" => "critical",
            "color" => "ff0000"
-         }]
+         }].map { |h| Hashie::Mash.new h }
       }
 
       before :each do
@@ -201,5 +191,104 @@ module OctoHerder
         end
       end
     end
+
+    context "when updating master repository" do
+      let (:master) { "my/master" }
+      let (:repositories) { ["my/slave"] }
+      let (:slave) { repositories.first }
+      let (:milestones) { [] }
+      let (:columns) { ["0 - Backlog"] }
+      let (:labels) { ["existing-red-label", "new-blue-label"] }
+      let (:conf) {
+        Configuration.new master, repositories, milestones, columns, labels
+      }
+
+      before :each do
+        connection.stub(:add_label)
+        connection.stub(:update_label)
+        connection.stub(:labels).and_return([])
+
+      end
+
+      it "should add missing columns" do
+        connection.should_receive(:add_label).with(an_instance_of(Octokit::Repository), "0 - Backlog", {color: "cccccc"})
+
+        conf.update_labels connection
+      end
+    end
+
+    context "when updating linked repositories" do
+      let (:master) { "my/master" }
+      let (:repositories) { ["my/slave"] }
+      let (:slave) { repositories.first }
+      let (:milestones) { [] }
+      let (:columns) { [] }
+      let (:labels) { ["existing-red-label", "new-blue-label"] }
+      let (:conf) {
+        Configuration.new master, repositories, milestones, columns, labels
+      }
+
+      before :each do
+       connection.stub(:add_label)
+        connection.stub(:update_label)
+        connection.stub(:labels) { |repo|
+          repo_name = "#{repo.username}/#{repo.name}"
+          case repo_name
+          when master then master_labels
+          when slave then slave_labels
+          else fail("Unknown repository #{repo_name}")
+          end
+        }
+      end
+
+      context "with existing labels" do
+        let (:master_labels) {
+          [{
+             "url" => "https =>//api.github.com/repos/me/mine/labels/existing-red-label",
+             "name" => "existing-red-label",
+             "color" => "ff0000"
+           }].map { |h| Hashie::Mash.new h }
+        }
+
+        let (:slave_labels) {
+          [{
+             "url" => "https =>//api.github.com/repos/me/mine/labels/existing-red-label",
+             "name" => "existing-red-label",
+             "color" => "cccccc"
+           }].map { |h| Hashie::Mash.new h }
+        }
+
+        it "should update labels in a linked repo" do
+          connection.should_receive(:update_label).with(an_instance_of(Octokit::Repository), master_labels.first.name, {color: 'ff0000'})
+
+          conf.update_labels connection
+        end
+      end
+
+      context "with missing labels" do
+        let (:columns) { ["0 - Backlog"] }
+        let (:master_labels) {
+          [{
+             "url" => "https =>//api.github.com/repos/me/mine/labels/new-blue-label",
+             "name" => "new-blue-label",
+             "color" => "0000ff"
+           },
+           { # Huboard column tags
+             "url" => "https =>//api.github.com/repos/me/mine/labels/0 - Backlog",
+             "name" => "0 - Backlog",
+             "color" => "cccccc"
+           }].map { |h| Hashie::Mash.new h }
+        }
+
+        let (:slave_labels) { [] }
+
+        it "should add labels to a linked repo with matching colour" do
+          connection.should_receive(:add_label).with(an_instance_of(Octokit::Repository), "new-blue-label", {color: "0000ff"})
+          connection.should_receive(:add_label).with(an_instance_of(Octokit::Repository), "0 - Backlog", {color: "cccccc"})
+
+          conf.update_labels connection
+        end
+      end
+    end
   end
 end

spec/data/sample.yml

@@ -28,4 +28,5 @@ labels:
   - 3
   - 5
   - 8
-  - 13
+  - 13
+  - critical

spec/spec_helper.rb

@@ -1,3 +1,6 @@
+require 'ostruct'
+require 'rspec'
+require 'data/sample-github-responses'
 require 'simplecov'
 SimpleCov.start