Clean up roulette testingrefactor-roulette-testing

This factors out the IO into its own testable module.

The advantage of this is that we can easily instantiate versions of the
plugin that have different team sources, without extensive mocking.
This proved to be necessary to test an update to the `roulette.json`
production in the handbook.

2 files changed, 115 insertions, 57 deletions

diff --git a/spec/lib/gitlab/danger/roulette_spec.rb b/spec/lib/gitlab/danger/roulette_spec.rb
index 121c5d8ecd9..40908556207 100644
--- a/spec/lib/gitlab/danger/roulette_spec.rb
+++ b/spec/lib/gitlab/danger/roulette_spec.rb
@@ -6,81 +6,69 @@ require 'webmock/rspec'
 require 'gitlab/danger/roulette'
 
 describe Gitlab::Danger::Roulette do
-  let(:teammate_json) do
-    <<~JSON
+  class MockRoulette
+    include Gitlab::Danger::Roulette
+  end
+
+  let(:teammate_data) do
     [
       {
-        "username": "in-gitlab-ce",
-        "name": "CE maintainer",
-        "projects":{ "gitlab-ce": "maintainer backend" }
+        "username" => "in-gitlab-ce",
+        "name" => "CE maintainer",
+        "projects" => { "gitlab-ce" => "maintainer backend" }
       },
       {
-        "username": "in-gitlab-ee",
-        "name": "EE reviewer",
-        "projects":{ "gitlab-ee": "reviewer frontend" }
+        "username" => "in-gitlab-ee",
+        "name" => "EE reviewer",
+        "projects" => { "gitlab-ee" => "reviewer frontend" }
       }
     ]
-    JSON
+  end
+
+  before do
+    allow(roulette).to receive(:roulette_data) { teammate_data }
   end
 
   let(:ce_teammate_matcher) do
-    satisfy do |teammate|
-      teammate.username == 'in-gitlab-ce' &&
-        teammate.name == 'CE maintainer' &&
-        teammate.projects == { 'gitlab-ce' => 'maintainer backend' }
-    end
+    have_attributes(
+      username: 'in-gitlab-ce',
+      name: 'CE maintainer',
+      projects: { 'gitlab-ce' => 'maintainer backend' })
   end
 
   let(:ee_teammate_matcher) do
-    satisfy do |teammate|
-      teammate.username == 'in-gitlab-ee' &&
-        teammate.name == 'EE reviewer' &&
-        teammate.projects == { 'gitlab-ee' => 'reviewer frontend' }
-    end
+    have_attributes(
+      username: 'in-gitlab-ee',
+      name: 'EE reviewer',
+      projects: { 'gitlab-ee' => 'reviewer frontend' })
   end
 
-  subject(:roulette) { Object.new.extend(described_class) }
+  subject(:roulette) { MockRoulette.new }
 
+  # We don't need to test that `http_get_json` does what it says it does - it
+  # is not our code after all. Since we are propagating errors, we just need to
+  # make sure we don't swallow them.
   describe '#team' do
     subject(:team) { roulette.team }
 
-    context 'HTTP failure' do
-      before do
-        WebMock
-          .stub_request(:get, described_class::ROULETTE_DATA_URL)
-          .to_return(status: 404)
-      end
-
-      it 'raises a pretty error' do
-        expect { team }.to raise_error(/Failed to read/)
-      end
-    end
-
-    context 'JSON failure' do
-      before do
-        WebMock
-          .stub_request(:get, described_class::ROULETTE_DATA_URL)
-          .to_return(body: 'INVALID JSON')
+    context 'on error' do
+      let(:teammate_data) do
+        raise "BOOM!"
       end
 
-      it 'raises a pretty error' do
-        expect { team }.to raise_error(/Failed to parse/)
+      it 'propagates the error' do
+        expect { team }.to raise_error(/BOOM/)
       end
     end
 
     context 'success' do
-      before do
-        WebMock
-          .stub_request(:get, described_class::ROULETTE_DATA_URL)
-          .to_return(body: teammate_json)
-      end
-
       it 'returns an array of teammates' do
         is_expected.to contain_exactly(ce_teammate_matcher, ee_teammate_matcher)
       end
 
       it 'memoizes the result' do
-        expect(team.object_id).to eq(roulette.team.object_id)
+        expect(roulette).to receive(:roulette_data).at_most(:once)
+        expect(team).to eq(roulette.team)
       end
     end
   end
@@ -88,12 +76,6 @@ describe Gitlab::Danger::Roulette do
   describe '#project_team' do
     subject { roulette.project_team('gitlab-ce') }
 
-    before do
-      WebMock
-        .stub_request(:get, described_class::ROULETTE_DATA_URL)
-        .to_return(body: teammate_json)
-    end
-
     it 'filters team by project_name' do
       is_expected.to contain_exactly(ce_teammate_matcher)
     end
@@ -117,22 +99,29 @@ describe Gitlab::Danger::Roulette do
 
     it 'returns a random person' do
       persons = [person1, person2]
+      names = persons.map(&:username)
+      expect(subject).to receive(:out_of_office?)
+        .exactly(:once)
+        .and_call_original
 
-      selected = subject.spin_for_person(persons, random: Random.new)
-
-      expect(selected.username).to be_in(persons.map(&:username))
+      expect(spin(persons)).to have_attributes(username: be_in(names))
     end
 
     it 'excludes OOO persons' do
-      expect(subject.spin_for_person([ooo], random: Random.new)).to be_nil
+      expect(spin([ooo])).to be_nil
     end
 
     it 'excludes mr.author' do
-      expect(subject.spin_for_person([author], random: Random.new)).to be_nil
+      expect(subject).not_to receive(:out_of_office?)
+      expect(spin([author])).to be_nil
     end
 
     private
 
+    def spin(people)
+      subject.spin_for_person(people, random: Random.new)
+    end
+
     def stub_person_message(person, message)
       body = { message: message }.to_json
 
diff --git a/spec/lib/gitlab/get_json_spec.rb b/spec/lib/gitlab/get_json_spec.rb
new file mode 100644
index 00000000000..48dc6cf192e
--- /dev/null
+++ b/spec/lib/gitlab/get_json_spec.rb
@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'ostruct'
+
+require 'gitlab/get_json'
+
+describe Gitlab::GetJSON do
+  subject do
+    mod = described_class
+    Class.new { include mod }.new
+  end
+
+  before do
+    allow(Faraday).to receive(:get) { response }
+  end
+
+  context 'the response is successful' do
+    let(:response) do
+      OpenStruct.new(success?: true, body: data.to_json)
+    end
+
+    let(:data) do
+      {
+        a: true,
+        b: 3.14159,
+        c: %w[one two three]
+      }
+    end
+
+    it 'returns the data back to us' do
+      expect(subject.http_get_json('some-url')).to eq data.transform_keys(&:to_s)
+    end
+  end
+
+  context 'we cannot connect' do
+    let(:response) do
+      raise Faraday::ConnectionFailed, 'for some reason'
+    end
+
+    it 'raises an HTTPError' do
+      expect { subject.http_get_json('url') }.to raise_error(Gitlab::GetJSON::HTTPError)
+    end
+  end
+
+  context 'the response has bad JSON data' do
+    let(:response) do
+      OpenStruct.new(success?: true, body: 'I am not valid JSON')
+    end
+
+    it 'raises a JSONError' do
+      expect { subject.http_get_json('url') }.to raise_error(Gitlab::GetJSON::JSONError)
+    end
+  end
+
+  context 'the response is 404' do
+    let(:response) do
+      OpenStruct.new(success?: false, status: 'wibble')
+    end
+
+    it 'raises an HTTPError' do
+      expect { subject.http_get_json('wobble') }.to raise_error(Gitlab::GetJSON::HTTPError)
+    end
+
+    it 'raises an error that mentions the status' do
+      expect { subject.http_get_json('wobble') }.to raise_error(/wibble/)
+    end
+  end
+end