From f7ab8cc504b8ccae2cc79ede080d8f594b9935d8 Mon Sep 17 00:00:00 2001
From: Beth Skurrie <beth@bethesque.com>
Date: Fri, 29 May 2020 09:27:14 +1000
Subject: [PATCH] feat(can-i-deploy): experimental - add a warning message if
 there are interactions missing verification test results.

---
 .../api/decorators/reason_decorator.rb        |  17 +++
 lib/pact_broker/domain/verification.rb        |  13 ++
 .../matrix/deployment_status_summary.rb       |  24 +++-
 lib/pact_broker/matrix/reason.rb              |   9 ++
 lib/pact_broker/pacts/content.rb              |  28 +++-
 script/foo-bar-verification.json              |   4 +-
 script/foo-bar.json                           |  11 ++
 .../api/decorators/reason_decorator_spec.rb   |  19 ++-
 .../matrix/deployment_status_summary_spec.rb  |   8 +-
 .../pact_broker/matrix/integration_spec.rb    |  43 ++++++
 spec/lib/pact_broker/pacts/content_spec.rb    | 125 ++++++++++++++++++
 11 files changed, 294 insertions(+), 7 deletions(-)

diff --git a/lib/pact_broker/api/decorators/reason_decorator.rb b/lib/pact_broker/api/decorators/reason_decorator.rb
index bfa9232d0..0804f7b9c 100644
--- a/lib/pact_broker/api/decorators/reason_decorator.rb
+++ b/lib/pact_broker/api/decorators/reason_decorator.rb
@@ -22,6 +22,11 @@ def to_s
             "There are no missing dependencies"
           when PactBroker::Matrix::Successful
             "All required verification results are published and successful"
+          when PactBroker::Matrix::InteractionsMissingVerifications
+            descriptions = reason.interactions.collect do | interaction |
+              interaction_description(interaction)
+            end.join('; ')
+            "WARNING: Although the verification was reported as successful, the results for #{reason.consumer_selector.description} and #{reason.provider_selector.description} may be missing tests for the following interactions: #{descriptions}"
           else
             reason
           end
@@ -44,6 +49,18 @@ def version_does_not_exist_description selector
             ""
           end
         end
+
+        # TODO move this somewhere else
+        def interaction_description(interaction)
+          if interaction['providerState'] && interaction['providerState'] != ''
+            "#{interaction['description']} given #{interaction['providerState']}"
+          elsif interaction['providerStates'] && interaction['providerStates'].is_a?(Array) && interaction['providerStates'].any?
+            provider_states = interaction['providerStates'].collect{ |ps| ps['name'] }.compact.join(', ')
+            "#{interaction['description']} given #{provider_states}"
+          else
+            interaction['description']
+          end
+        end
       end
     end
   end
diff --git a/lib/pact_broker/domain/verification.rb b/lib/pact_broker/domain/verification.rb
index 21b4581f9..acb2cc590 100644
--- a/lib/pact_broker/domain/verification.rb
+++ b/lib/pact_broker/domain/verification.rb
@@ -2,6 +2,7 @@
 require 'sequel'
 require 'pact_broker/repositories/helpers'
 require 'pact_broker/tags/tag_with_latest_flag'
+require 'pact_broker/pacts/content'
 
 
 module PactBroker
@@ -83,6 +84,18 @@ def provider_version_tag_names
       def latest_pact_publication
         pact_version.latest_pact_publication
       end
+
+      def interactions_missing_test_results
+        @interactions_missing_test_results ||= pact_content_with_test_results.interactions_missing_test_results
+      end
+
+      def all_interactions_missing_test_results?
+        pact_content_with_test_results.interactions.count == pact_content_with_test_results.interactions_missing_test_results.count
+      end
+
+      def pact_content_with_test_results
+        @pact_content_with_test_results = PactBroker::Pacts::Content.from_json(pact_version.content).with_test_results(test_results)
+      end
     end
 
     Verification.plugin :timestamps
diff --git a/lib/pact_broker/matrix/deployment_status_summary.rb b/lib/pact_broker/matrix/deployment_status_summary.rb
index a019f1247..29dcc91ba 100644
--- a/lib/pact_broker/matrix/deployment_status_summary.rb
+++ b/lib/pact_broker/matrix/deployment_status_summary.rb
@@ -31,7 +31,7 @@ def deployable?
       end
 
       def reasons
-        error_messages.any? ? error_messages : success_messages
+        error_messages.any? ? error_messages + warning_messages : success_messages + warning_messages
       end
 
       private
@@ -51,6 +51,10 @@ def error_messages
         end
       end
 
+      def warning_messages
+        warnings_for_missing_interactions
+      end
+
       def specified_selectors_that_do_not_exist
         resolved_selectors.select(&:specified_version_that_does_not_exist?)
       end
@@ -182,6 +186,24 @@ def dummy_selectors_from_rows
           [dummy_consumer_selector, dummy_provider_selector]
         end.flatten
       end
+
+      # experimental
+      def warnings_for_missing_interactions
+        rows.select(&:success).collect do | row |
+          begin
+            if row.verification.interactions_missing_test_results.any? && !row.verification.all_interactions_missing_test_results?
+              InteractionsMissingVerifications.new(selector_for(row.consumer_name), selector_for(row.provider_name), row.verification.interactions_missing_test_results)
+            end
+          rescue StandardError => e
+            log_error(e, "Error determining if there were missing interaction verifications")
+            nil
+          end
+        end.compact.tap { |it| report_missing_interaction_verifications(it) if it.any? }
+      end
+
+      def report_missing_interaction_verifications(messages)
+        logger.warn("Interactions missing verifications", messages)
+      end
     end
   end
 end
diff --git a/lib/pact_broker/matrix/reason.rb b/lib/pact_broker/matrix/reason.rb
index 87a6a7a7a..6600a59a7 100644
--- a/lib/pact_broker/matrix/reason.rb
+++ b/lib/pact_broker/matrix/reason.rb
@@ -73,5 +73,14 @@ class Successful < Reason
     # provider verifications.
     class NoDependenciesMissing < Reason
     end
+
+    class InteractionsMissingVerifications < ErrorReasonWithTwoSelectors
+      attr_reader :interactions
+
+      def initialize(consumer_selector, provider_selector, interactions)
+        super(consumer_selector, provider_selector)
+        @interactions = interactions
+      end
+    end
   end
 end
diff --git a/lib/pact_broker/pacts/content.rb b/lib/pact_broker/pacts/content.rb
index 1a5d7e6f6..cfa5ec0e8 100644
--- a/lib/pact_broker/pacts/content.rb
+++ b/lib/pact_broker/pacts/content.rb
@@ -31,14 +31,25 @@ def sort
         Content.from_hash(SortContent.call(pact_hash))
       end
 
+      def interactions_missing_test_results
+        interactions.reject do | interaction |
+          interaction['tests']&.any?
+        end
+      end
+
       def with_test_results(test_results)
+        tests = test_results && test_results['tests']
+        if tests.nil? || !tests.is_a?(Array) || tests.empty?
+          tests = []
+        end
+
         new_pact_hash = pact_hash.dup
         if interactions && interactions.is_a?(Array)
-          new_pact_hash['interactions'] = merge_verification_results(interactions, test_results)
+          new_pact_hash['interactions'] = merge_verification_results(interactions, tests)
         end
 
         if messages && messages.is_a?(Array)
-          new_pact_hash['messages'] = merge_verification_results(messages, test_results)
+          new_pact_hash['messages'] = merge_verification_results(messages, tests)
         end
         Content.from_hash(new_pact_hash)
       end
@@ -107,6 +118,19 @@ def add_ids(interactions, overwrite_existing_id)
           end
         end
       end
+
+      def merge_verification_results(interactions, tests)
+        interactions.collect(&:dup).collect do | interaction |
+          interaction['tests'] = tests.select do | test |
+            test_is_for_interaction(interaction, test)
+          end
+          interaction
+        end
+      end
+
+      def test_is_for_interaction(interaction, test)
+        test.is_a?(Hash) && interaction.is_a?(Hash) && test['interactionDescription'] == interaction['description'] && test['interactionProviderState'] == interaction['providerState']
+      end
     end
   end
 end
diff --git a/script/foo-bar-verification.json b/script/foo-bar-verification.json
index 998ef14a3..dc23c8138 100644
--- a/script/foo-bar-verification.json
+++ b/script/foo-bar-verification.json
@@ -2,8 +2,10 @@
   "success": true,
   "providerApplicationVersion": "1.0.0",
   "testResults": {
-    "examples": [
+    "tests": [
       {
+        "interactionDescription": "a request for something",
+        "interactionProviderState": null,
         "description": "has status code 200",
         "file_path": "/redacted/.gem/ruby/2.4.1/gems/pact-1.17.0/lib/pact/provider/rspec.rb",
         "full_description": "Verifying a pact between me and they Greeting with GET / returns a response which has status code 200",
diff --git a/script/foo-bar.json b/script/foo-bar.json
index ba5048dba..2359c4d04 100644
--- a/script/foo-bar.json
+++ b/script/foo-bar.json
@@ -17,6 +17,17 @@
         "status": 200,
         "body" : "something"
       }
+    },{
+      "description" : "a request for something else",
+      "providerState": null,
+      "request": {
+        "method": "get",
+        "path" : "/something"
+      },
+      "response": {
+        "status": 200,
+        "body" : "something"
+      }
     }
   ]
 }
diff --git a/spec/lib/pact_broker/api/decorators/reason_decorator_spec.rb b/spec/lib/pact_broker/api/decorators/reason_decorator_spec.rb
index f1cd3606d..6cc09cc90 100644
--- a/spec/lib/pact_broker/api/decorators/reason_decorator_spec.rb
+++ b/spec/lib/pact_broker/api/decorators/reason_decorator_spec.rb
@@ -10,7 +10,7 @@ module Decorators
 
         describe "the number of Reason classes" do
           it "is 9 - add another spec here if a new Reason is added" do
-            expect(REASON_CLASSES.size).to eq 9
+            expect(REASON_CLASSES.size).to eq 10
           end
         end
 
@@ -52,6 +52,23 @@ module Decorators
 
             its(:to_s) { is_expected.to eq "All required verification results are published and successful" }
           end
+
+          context "when the reason is PactBroker::Matrix::InteractionsMissingVerifications" do
+            let(:reason) { PactBroker::Matrix::InteractionsMissingVerifications.new(consumer_selector, provider_selector, interactions) }
+            let(:interactions) do
+              [
+                {
+                  "description" => "desc1",
+                  "providerState" => "p2"
+                },{
+                  "description" => "desc1",
+                  "providerStates" => [ { "name" => "desc3"}, { "name" => "desc4"} ]
+                }
+              ]
+            end
+
+            its(:to_s) { is_expected.to eq "WARNING: Although the verification was reported as successful, the results for version 2 of Foo and version 6 of Bar may be missing tests for the following interactions: desc1 given p2; desc1 given desc3, desc4" }
+          end
         end
       end
     end
diff --git a/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb b/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb
index c6832037f..df00e3911 100644
--- a/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb
+++ b/spec/lib/pact_broker/matrix/deployment_status_summary_spec.rb
@@ -29,7 +29,8 @@ module Matrix
             provider_name: bar.name,
             provider_id: bar.id,
             success: row_1_success,
-            pacticipant_names: [foo.name, bar.name]
+            pacticipant_names: [foo.name, bar.name],
+            verification: verification_1
           )
         end
 
@@ -46,7 +47,8 @@ module Matrix
             provider_name: baz.name,
             provider_id: baz.id,
             success: true,
-            pacticipant_names: [foo.name, baz.name]
+            pacticipant_names: [foo.name, baz.name],
+            verification: verification_2
           )
         end
 
@@ -65,6 +67,8 @@ module Matrix
         let(:foo_version) { double('foo version', number: "ddec8101dabf4edf9125a69f9a161f0f294af43c", id: 10)}
         let(:bar_version) { double('bar version', number: "14131c5da3abf323ccf410b1b619edac76231243", id: 11)}
         let(:baz_version) { double('baz version', number: "4ee06460f10e8207ad904fa9fa6c4842e462ab59", id: 12)}
+        let(:verification_1) { double('verification 1', interactions_missing_test_results: []) }
+        let(:verification_2) { double('verification 2', interactions_missing_test_results: []) }
 
         let(:resolved_selectors) do
           [
diff --git a/spec/lib/pact_broker/matrix/integration_spec.rb b/spec/lib/pact_broker/matrix/integration_spec.rb
index b53dfa776..98230822f 100644
--- a/spec/lib/pact_broker/matrix/integration_spec.rb
+++ b/spec/lib/pact_broker/matrix/integration_spec.rb
@@ -416,6 +416,49 @@ module Matrix
             expect(subject.deployment_status_summary).to be_deployable
           end
         end
+
+        describe "when verification results are published missing tests for some interactions" do
+          let(:pact_content) do
+            {
+              "interactions" => [
+                {
+                  "description" => "desc1"
+                },{
+                  "description" => "desc2"
+                }
+              ]
+            }
+          end
+
+          let(:verification_tests) do
+            [
+              {
+                "interactionDescription" => "desc1"
+              }
+            ]
+          end
+
+          before do
+            td.create_consumer("Foo")
+              .create_provider("Bar")
+              .create_consumer_version
+              .create_pact(json_content: pact_content.to_json)
+              .create_verification(provider_version: "1", test_results: { tests: verification_tests })
+          end
+
+          let(:selectors) do
+            [
+              UnresolvedSelector.new(pacticipant_name: "Foo", latest: true),
+              UnresolvedSelector.new(pacticipant_name: "Bar", latest: true)
+            ]
+          end
+
+          let(:options) { { latestby: "cvpv"} }
+
+          it "should include a warning" do
+            expect(subject.deployment_status_summary.reasons.last).to be_a(PactBroker::Matrix::InteractionsMissingVerifications)
+          end
+        end
       end
     end
   end
diff --git a/spec/lib/pact_broker/pacts/content_spec.rb b/spec/lib/pact_broker/pacts/content_spec.rb
index 69d8e3645..624b0451b 100644
--- a/spec/lib/pact_broker/pacts/content_spec.rb
+++ b/spec/lib/pact_broker/pacts/content_spec.rb
@@ -227,6 +227,131 @@ module Pacts
           its(:pact_specification_version) { is_expected.to eq nil }
         end
       end
+
+      describe "with_test_results" do
+        let(:test_results) do
+          {
+            "tests" => [
+              {
+                "interactionProviderState" => "ps1",
+                "interactionDescription" => "desc1"
+              },{
+                "interactionProviderState" => "ps1",
+                "interactionDescription" => "desc1"
+              }
+            ]
+          }
+        end
+
+        let(:pact_content) do
+          {
+            "interactions" => [
+              {
+               "providerState" => "ps1",
+               "description" => "desc1",
+              },
+              {
+               "providerState" => "ps2",
+               "description" => "desc2",
+              }
+            ]
+          }
+        end
+
+        subject { Content.from_hash(pact_content).with_test_results(test_results) }
+
+        let(:merged) do
+          {
+            "interactions" => [
+              {
+               "providerState" => "ps1",
+               "description" => "desc1",
+               "tests" => [
+                {
+                  "interactionProviderState" => "ps1",
+                  "interactionDescription" => "desc1",
+                },{
+                  "interactionProviderState" => "ps1",
+                  "interactionDescription" => "desc1",
+                }
+               ]
+              },{
+               "providerState" => "ps2",
+               "description" => "desc2",
+               "tests" => []
+              }
+            ]
+          }
+        end
+
+        let(:merged_with_empty_tests) do
+          {
+            "interactions" => [
+              {
+               "providerState" => "ps1",
+               "description" => "desc1",
+               "tests" => []
+              },{
+               "providerState" => "ps2",
+               "description" => "desc2",
+               "tests" => []
+              }
+            ]
+          }
+        end
+
+        it "merges the contents with the results" do
+          expect(subject.to_hash).to eq merged
+        end
+
+        it "returns interactions without test results" do
+          expect(subject.interactions_missing_test_results.count).to eq 1
+        end
+
+        context "with nil test results" do
+          let(:test_results) { nil }
+
+          it "does not blow up" do
+            expect(subject.to_hash).to eq merged_with_empty_tests
+          end
+
+          it "returns interactions without test results" do
+            expect(subject.interactions_missing_test_results.count).to eq 2
+          end
+        end
+
+        context "with nil tests" do
+          let(:test_results) { {} }
+
+          it "does not blow up" do
+            expect(subject.to_hash).to eq merged_with_empty_tests
+          end
+        end
+
+        context "with empty tests" do
+          let(:test_results) { { "tests" => [] } }
+
+          it "does not blow up" do
+            expect(subject.to_hash).to eq merged_with_empty_tests
+          end
+        end
+
+        context "with tests not an array" do
+          let(:test_results) { { "tests" => {} } }
+
+          it "does not blow up" do
+            expect(subject.to_hash).to eq merged_with_empty_tests
+          end
+        end
+
+        context "with tests an array of not hashes" do
+          let(:test_results) { { "tests" => [1] } }
+
+          it "does not blow up" do
+            expect(subject.to_hash).to eq merged_with_empty_tests
+          end
+        end
+      end
     end
   end
 end