Add tests for TestFlight methods in spaceship and fastlane_core

fastlane.gemspec

@@ -80,4 +80,5 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'rb-readline' # https://github.com/deivid-rodriguez/byebug/issues/289#issuecomment-251383465
   spec.add_development_dependency 'rest-client', '~> 1.6.7'
   spec.add_development_dependency 'fakefs', '~> 0.8.1'
+  spec.add_development_dependency 'sinatra', '~> 1.4.8'
 end

fastlane_core/lib/fastlane_core/build_watcher.rb

@@ -1,36 +1,54 @@
 module FastlaneCore
   class BuildWatcher
-    # @return The build we waited for. This method will always return a build
-    def self.wait_for_build_processing_to_be_complete(app_id: nil, platform: nil)
-      # First, find the train and build version we want to watch for
-      processing_builds = Spaceship::TestFlight::Build.all_processing_builds(app_id: app_id, platform: platform)
+    class << self
+      # @return The build we waited for. This method will always return a build
+      def wait_for_build_processing_to_be_complete(app_id: nil, platform: nil)
+        # First, find the train and build version we want to watch for
+        watched_build = watching_build(app_id: app_id, platform: platform)
+        UI.crash!("Could not find a build for app: #{app_id} on platform: #{platform}") if watched_build.nil?
 
-      watching_build = processing_builds.sort_by(&:upload_date).last # either it's still processing
-      watching_build ||= Spaceship::TestFlight::Build.latest(app_id: app_id, platform: platform) # or we fallback to the most recent uplaod
+        loop do
+          matched_build = matching_build(watched_build: watched_build, app_id: app_id, platform: platform)
 
-      loop do
-        UI.message("Waiting for iTunes Connect to finish processing the new build (#{watching_build.train_version} - #{watching_build.build_version})")
+          report_status(build: matched_build)
 
+          if matched_build && matched_build.processed?
+            return matched_build
+          end
+
+          sleep 10
+        end
+      end
+
+      private
+
+      def watching_build(app_id: nil, platform: nil)
+        processing_builds = Spaceship::TestFlight::Build.all_processing_builds(app_id: app_id, platform: platform)
+
+        watched_build = processing_builds.sort_by(&:upload_date).last
+        watched_build || Spaceship::TestFlight::Build.latest(app_id: app_id, platform: platform)
+      end
+
+      def matching_build(watched_build: nil, app_id: nil, platform: nil)
+        matched_builds = Spaceship::TestFlight::Build.builds_for_train(app_id: app_id, platform: platform, train_version: watched_build.train_version)
+        matched_builds.find { |build| build.build_version == watched_build.build_version }
+      end
+
+      def report_status(build: nil)
         # Due to iTunes Connect, builds disappear from the build list alltogether
         # after they finished processing. Before returning this build, we have to
         # wait for the build to appear in the build list again
         # As this method is very often used to wait for a build, and then do something
         # with it, we have to be sure that the build actually is ready
-
-        matching_builds = Spaceship::TestFlight::Build.builds_for_train(app_id: app_id, platform: platform, train_version: watching_build.train_version)
-        matching_build = matching_builds.find { |build| build.build_version == watching_build.build_version }
-
-        if matching_build.nil?
+        if build.nil?
           UI.message("Build doesn't show up in the build list any more, waiting for it to appear again")
-        elsif matching_build.active?
-          UI.success("Build #{matching_build.train_version} - #{matching_build.build_version} is already being tested")
-          return matching_build
-        elsif matching_build.ready_to_submit? || matching_build.export_compliance_missing?
-          UI.success("Successfully finished processing the build #{matching_build.train_version} - #{matching_build.build_version}")
-          return matching_build
+        elsif build.active?
+          UI.success("Build #{build.train_version} - #{build.build_version} is already being tested")
+        elsif build.ready_to_submit? || build.export_compliance_missing?
+          UI.success("Successfully finished processing the build #{build.train_version} - #{build.build_version}")
+        else
+          UI.message("Waiting for iTunes Connect to finish processing the new build (#{build.train_version} - #{build.build_version})")
         end
-
-        sleep 10
       end
     end
   end

fastlane_core/spec/build_watcher_spec.rb

@@ -0,0 +1,154 @@
+require 'spec_helper'
+
+describe FastlaneCore::BuildWatcher do
+  context '.wait_for_build_processing_to_be_complete' do
+    let(:processing_build) do
+      double(
+        'Processing Build',
+        processed?: false,
+        active?: false,
+        ready_to_submit?: false,
+        export_compliance_missing?: false,
+        train_version: '1.0',
+        build_version: '1',
+        upload_date: 1
+      )
+    end
+    let(:old_processing_build) do
+      double(
+        'Old Processing Build',
+        processed?: false,
+        active?: false,
+        ready_to_submit?: false,
+        export_compliance_missing?: false,
+        train_version: '1.0',
+        build_version: '0',
+        upload_date: 0
+      )
+    end
+    let(:active_build) do
+      double(
+        'Active Build',
+        processed?: true,
+        active?: true,
+        ready_to_submit?: false,
+        export_compliance_missing?: false,
+        train_version: '1.0',
+        build_version: '1',
+        upload_date: 1
+      )
+    end
+    let(:ready_build) do
+      double(
+        'Ready Build',
+        processed?: true,
+        active?: false,
+        ready_to_submit?: true,
+        export_compliance_missing?: false,
+        train_version: '1.0',
+        build_version: '1',
+        upload_date: 1
+      )
+    end
+    let(:export_compliance_required_build) do
+      double(
+        'Export Compliance Required Build',
+        processed?: true,
+        active?: false,
+        ready_to_submit?: false,
+        export_compliance_missing?: true,
+        train_version: '1.0',
+        build_version: '1',
+        upload_date: 1
+      )
+    end
+
+    it 'returns an already-active build' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([])
+      expect(Spaceship::TestFlight::Build).to receive(:latest).and_return(active_build)
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([active_build])
+
+      expect(UI).to receive(:success).with("Build #{active_build.train_version} - #{active_build.build_version} is already being tested")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(active_build)
+    end
+
+    it 'returns a ready to submit build' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([])
+      expect(Spaceship::TestFlight::Build).to receive(:latest).and_return(ready_build)
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([ready_build])
+
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{ready_build.train_version} - #{ready_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(ready_build)
+    end
+
+    it 'returns a export-compliance-missing build' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([])
+      expect(Spaceship::TestFlight::Build).to receive(:latest).and_return(export_compliance_required_build)
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([export_compliance_required_build])
+
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{export_compliance_required_build.train_version} - #{export_compliance_required_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(export_compliance_required_build)
+    end
+
+    it 'waits when a build is still processing' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([processing_build])
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([processing_build], [ready_build])
+      expect(FastlaneCore::BuildWatcher).to receive(:sleep)
+
+      expect(UI).to receive(:message).with("Waiting for iTunes Connect to finish processing the new build (#{ready_build.train_version} - #{ready_build.build_version})")
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{ready_build.train_version} - #{ready_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(ready_build)
+    end
+
+    it 'waits when the build disappears' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([processing_build])
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([], [ready_build])
+      expect(FastlaneCore::BuildWatcher).to receive(:sleep)
+
+      expect(UI).to receive(:message).with("Build doesn't show up in the build list any more, waiting for it to appear again")
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{ready_build.train_version} - #{ready_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(ready_build)
+    end
+
+    it 'watches the latest build when more than one build is processing' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([processing_build, old_processing_build])
+      # Mock `:builds_for_train` to return a build in the ready state because this will terminate the wait loop.
+      # Note that ready_build and processing_build have same build train and build number.
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([ready_build])
+
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{ready_build.train_version} - #{ready_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(ready_build)
+    end
+
+    it 'watches the latest build when no builds are processing' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([])
+      expect(Spaceship::TestFlight::Build).to receive(:latest).and_return(ready_build)
+      expect(Spaceship::TestFlight::Build).to receive(:builds_for_train).and_return([ready_build])
+
+      expect(UI).to receive(:success).with("Successfully finished processing the build #{ready_build.train_version} - #{ready_build.build_version}")
+      found_build = FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios)
+
+      expect(found_build).to eq(ready_build)
+    end
+
+    it 'crashes if it cannot find a build to watch' do
+      expect(Spaceship::TestFlight::Build).to receive(:all_processing_builds).and_return([])
+      expect(Spaceship::TestFlight::Build).to receive(:latest).and_return(nil)
+
+      expect(UI).to receive(:crash!).with("Could not find a build for app: some-app-id on platform: ios").and_call_original
+      expect { FastlaneCore::BuildWatcher.wait_for_build_processing_to_be_complete(app_id: 'some-app-id', platform: :ios) }.to raise_error(FastlaneCore::Interface::FastlaneCrash)
+    end
+  end
+end

spaceship/docs/TestFlightTesting.md

@@ -0,0 +1,119 @@
+Testing `Spaceship::TestFlight`
+===================
+(But we would love for all of the _spaceship_ tests to be like this 😀)
+
+## Usage
+To run the tests, in your terminal run:
+
+```
+bundle exec rspec spaceship/spec
+```
+
+## Overview
+
+Spaceship wraps various APIs using the following pattern:
+
+A simple `client` and various data models, usually subclassed from a `Base` model (e.g. Spaceship::TestFlight::Base)
+The `client` is responsible for making HTTP requests for a given API or domain. It should be very simple and have no logic.
+It is only responsible for creating the request and parsing the response. The best practice is for each method to have a single request and return the data from the response.
+
+The data models generally map to a REST resource or some logical grouping of data. Each data model has an instance of `client` which it can use to put or get data. It should encapsulate all interactions with the API, so other _fastlane_ tools interface with the data models, and not the `client` directly.
+
+
+## Adding Tests
+### Models
+
+Since the data models expect the client to return JSON data as a Ruby hash, we can reasonably mock the client response using RSpec doubles. We should *not* rely on HTTP fixtures because they are brittle, introduce global state, are not easily decomposable and are difficult to maintain. Instead, use the helper method `mock_client_response` to set up the expected data returned by the API. This design also leads the client to be as thin and logic-free as possible.
+
+Defining the response near the test site makes it easy to understand and maintain. Try not to include any more data than is necessary for the spec to pass.
+
+**Examples:**
+
+At the top of your data model spec, set the client to be a `mock_client`:
+
+```ruby
+describe Spaceship::TestFlight::Tester do
+  let(:mock_client) { double('MockClient') }
+  before do
+    Spaceship::TestFlight::Base.client = mock_client
+  end
+end
+```
+Now, anytime we use a data model that is a subclass of `Spaceship::TestFlight::Base`, it has a `client` that is our mock.
+
+We then configure the response for a given client method using the `mock_client_response` method defined in `spaceship/spec/spec_helper.rb` which can be required by `require 'spec_helper`. This method is defined within an RSpec configuration block:
+
+```ruby
+before do
+  mock_client_response(:get_tester, with: { tester_id: 1 }) do
+    {
+      id: 1,
+      name: 'Mr. Tester'
+    }
+  end
+end
+```
+
+The first parameter is the name of the method we are mocking, and `with:` parameter specifies required parameters to that method. If you don't give it a `with:`, the mock will accept any parameters. The block is the return value of calling `client.get_tester`.
+
+Now we can test our data model method that uses the client:
+
+```ruby
+it 'finds the test by id' do
+  tester = Spaceship::TestFlight::Tester.find(1)
+  expect(tester.name).to eq('Mr. Tester')
+end
+```
+
+Collection methods:
+
+```
+context '.all' do
+  it 'contains all of the builds across all build trains' do
+    builds = Spaceship::TestFlight::Build.all(app_id: 10, platform: 'ios')
+    expect(builds.size).to eq(3)
+    expect(builds.sample).to be_instance_of(Spaceship::TestFlight::Build)
+    expect(builds.map(&:train_version).uniq).to eq(['1.0', '1.1'])
+  end
+end
+```
+
+Instance methods:
+
+```
+context '#upload_date' do
+  it 'parses the string value' do
+    expect(build.upload_date).to eq(Time.utc(2017, 1, 1, 12))
+  end
+end
+```
+
+### Client
+**Examples:**
+
+GET:
+
+```
+context '#get_build_trains' do
+  it 'executes the request' do
+    MockAPI::TestFlightServer.get('/testflight/v2/providers/fake-team-id/apps/some-app-id/platforms/ios/trains') {}
+    subject.get_build_trains(app_id: app_id, platform: platform)
+    expect(WebMock).to have_requested(:get, 'https://itunesconnect.apple.com/testflight/v2/providers/fake-team-id/apps/some-app-id/platforms/ios/trains')
+  end
+end
+```
+
+PUT:
+
+```
+context '#add_group_to_build' do
+  it 'executes the request' do
+    MockAPI::TestFlightServer.put('/testflight/v2/providers/fake-team-id/apps/some-app-id/groups/fake-group-id/builds/fake-build-id') {}
+    subject.add_group_to_build(app_id: app_id, group_id: 'fake-group-id', build_id: 'fake-build-id')
+    expect(WebMock).to have_requested(:put, 'https://itunesconnect.apple.com/testflight/v2/providers/fake-team-id/apps/some-app-id/groups/fake-group-id/builds/fake-build-id')
+  end
+end
+```
+
+#### How client tests work
+The client adds routes to a [sinatra](http://www.sinatrarb.com/) server to receive and mock the responses that will be handled by `handle_response`.

spaceship/lib/spaceship/test_flight/base.rb

@@ -4,6 +4,18 @@ def self.client
       @client ||= Client.client_with_authorization_from(Spaceship::Tunes.client)
     end
 
+    ##
+    # Have subclasses inherit the client from their superclass
+    #
+    # Essentially, we are making a class-inheritable-accessor as described here:
+    # https://apidock.com/rails/v4.2.7/Class/class_attribute
+    def self.inherited(subclass)
+      this_class = self
+      subclass.define_singleton_method(:client) do
+        this_class.client
+      end
+    end
+
     def to_json
       raw_data.to_json
     end