CI error handling improvements

config/locales/server.en.yml

@@ -19,8 +19,8 @@ en:
       fixed:
         body: "%{plugin_name} has recovered from this issue."
     test:
-      error: "The tests are failing."
-      error_test: "The '%{test_name}' test is failing."
+      failed: "%{test_name} has failed."
+      failed_with_message: "%{test_name} has failed: %{message}"
     plugin:
       documentation_category_description: Documentation for the %{plugin_name} plugin.
       issues_category_title: Issues for the %{plugin_name} plugin.

lib/plugin_manager/plugin.rb

@@ -142,7 +142,7 @@ def self.set(plugin_name, attrs)
     new_attrs = update_repository_attrs(new_attrs[:url], new_attrs)
     PluginManagerStore.set(::PluginManager::NAMESPACE, plugin_name, new_attrs)
 
-    status_attrs = attrs.slice(:status, :test_status)
+    status_attrs = attrs.slice(:status, :test_status, :backtrace, :message)
     git = attrs.slice(*PluginManager::Plugin::Status.required_git_attrs)
 
     if status_attrs.present? && git.present?

lib/plugin_manager/test_host.rb

@@ -11,7 +11,9 @@ class ::PluginManager::TestHost
 
   attr_accessor :plugin,
                 :branch,
-                :discourse_branch
+                :discourse_branch,
+                :manager,
+                :test_error
 
   ## overide in child
   def status_path

lib/plugin_manager/test_host/github.rb

@@ -11,26 +11,68 @@ def repo_path
   end
 
   def status_path
-    "repos#{repo_path}/actions/runs?branch=#{@branch}&status=completed&per_page=1&page=1"
+    "repos#{repo_path}/actions/runs?branch=#{@branch}&status=completed&per_page=5&page=1"
   end
 
   def config_path
     "repos#{repo_path}/contents/#{@config}"
   end
 
+  def tests_workflow_name
+    "Discourse Plugin"
+  end
+
+  def check_runs_path(check_suite_id)
+    "repos#{repo_path}/check-suites/#{check_suite_id}/check-runs?status=completed"
+  end
+
+  def test_check_runs
+    %w(frontend_tests backend_tests)
+  end
+
+  def test_check_run?(run)
+    test_check_runs.any? { |tcr| run['name'].include?(tcr) }
+  end
+
+  def failed_run?(run)
+    run['conclusion'] === "failure"
+  end
+
+  def run_error_message(run)
+    message = I18n.t("plugin_manager.test.failed", test_name: run["name"])
+    return message unless run["output"] && run["output"]["annotations_url"]
+
+    annotations = @manager.request(nil, url: run["output"]["annotations_url"])
+    message = annotations.map { |a| a['message'] }.join(', ')
+    I18n.t("plugin_manager.test.failed_with_message", test_name: run["name"], message: message)
+  end
+
+  def build_test_error(runs)
+    message = runs.map { |r| run_error_message(r) }.join(', ')
+  end
+
   def get_status_from_response(response)
-    runs = response['workflow_runs']
-    return nil unless runs.present?
-    latest_run = runs.first
+    latest_run = response['workflow_runs'].find { |r| r["name"] === tests_workflow_name }
+    return nil unless latest_run.present?
 
     @test_sha = latest_run['head_sha']
     @test_branch = latest_run['head_branch']
     @test_name = latest_run['name']
     @test_url = latest_run['html_url']
+    @check_suite_id = latest_run['check_suite_id']
 
     if latest_run["conclusion"] === "success"
       PluginManager::TestManager.status[:passing]
     else
+      check_runs = @manager.request(check_runs_path(latest_run["check_suite_id"]))
+
+      if check_runs && check_runs['check_runs']
+        failing_test_runs = check_runs['check_runs'].select { |cr| test_check_run?(cr) && failed_run?(cr) }
+        return PluginManager::TestManager.status[:passing] if failing_test_runs.blank?
+
+        @test_error = build_test_error(failing_test_runs)
+      end
+
       PluginManager::TestManager.status[:failing]
     end
   end

lib/plugin_manager/test_manager.rb

@@ -32,14 +32,17 @@ def update(plugin_name)
     @host.plugin = @plugin
     @host.branch = branch
     @host.discourse_branch = discourse_branch
+    @host.manager = self
 
-    test_status = request_test_status
+    status_response = request(@host.status_path)
+    test_status = @host.get_status_from_response(status_response)
 
     if !test_status.nil?
       attrs = {}
       attrs[:test_status] = test_status
       attrs[:branch] = branch
       attrs[:discourse_branch] = discourse_branch
+      attrs[:message] = @host.test_error if @host.test_error
 
       PluginManager::Plugin.set(@plugin.name, attrs)
     end
@@ -53,23 +56,10 @@ def self.failing?(test_status)
     test_status == PluginManager::TestManager.status[:failing]
   end
 
-  protected
-
-  def request_test_status
-    status_path = @host.status_path
-
-    if status_path && response = request(status_path)
-      @host.get_status_from_response(response)
-    else
-      nil
-    end
-  end
-
-  def request(endpoint, opts = {})
-    connection = Excon.new(
-      "https://#{@host.domain}/#{endpoint}",
-      middlewares: Excon.defaults[:middlewares] + [Excon::Middleware::RedirectFollower]
-    )
+  def request(endpoint = nil, opts = {})
+    url = opts[:url] || "https://#{@host.domain}/#{endpoint}"
+    middlewares = Excon.defaults[:middlewares] + [Excon::Middleware::RedirectFollower]
+    connection = Excon.new(url, middlewares: middlewares)
 
     begin
       response = connection.request(opts)

spec/components/plugin_manager/test_manager_spec.rb

@@ -1,9 +1,34 @@
 # frozen_string_literal: true
 require_relative '../../plugin_helper'
 
+# workflow indexes
+# 0: "Discourse Plugin"
+
+# text check indexes
+# 0: "ci / linting"
+# 1: "ci / check_for_tests"
+# 2: "ci / frontend_tests"
+# 3: "ci / backend_tests"
+
 describe PluginManager::TestManager do
   let(:incompatible_plugin) { "incompatible_plugin" }
   let(:test_response_body) { File.read("#{fixture_dir}/github/runs.json") }
+  let(:test_checks_response_body) { File.read("#{fixture_dir}/github/check_runs.json") }
+  let(:test_annotations_response_body) { File.read("#{fixture_dir}/github/annotations.json") }
+  let(:subject) { described_class.new("github", plugin_branch, discourse_branch) }
+  let(:status) { PluginManager::Plugin::Status.get(compatible_plugin, plugin_branch, discourse_branch) }
+
+  def set_workflow(index, key, value)
+    test_response_json = JSON.parse(test_response_body)
+    test_response_json['workflow_runs'][index][key] = value
+    stub_github_test_request(JSON.generate(test_response_json))
+  end
+
+  def set_test_check(index, key, value)
+    test_checks_response_json = JSON.parse(test_checks_response_body)
+    test_checks_response_json['check_runs'][index][key] = value
+    stub_github_test_check_request(JSON.generate(test_checks_response_json))
+  end
 
   before do
     stub_github_user_request
@@ -13,19 +38,44 @@
     setup_test_plugin(compatible_plugin)
   end
 
-  it "updates plugin tests" do
-    manager = described_class.new("github", plugin_branch, discourse_branch)
-    manager.update(compatible_plugin)
+  it "does nothing if plugin is not using Discourse Plugin workflow" do
+    set_workflow(0, 'name', 'Plugin Tests')
+    subject.update(compatible_plugin)
+    expect(status.test_status).to eq(nil)
+  end
+
+  it "sets a passing test status when tests are passing" do
+    subject.update(compatible_plugin)
+    expect(status.test_status).to eq(described_class.status[:passing])
+  end
 
-    status = PluginManager::Plugin::Status.get(compatible_plugin, plugin_branch, discourse_branch)
+  it "sets a passing test status when linting is failing" do
+    set_workflow(0, 'conclusion', 'failure')
+    set_test_check(0, 'conclusion', 'failure')
+
+    subject.update(compatible_plugin)
     expect(status.test_status).to eq(described_class.status[:passing])
+  end
 
-    test_response_json = JSON.parse(test_response_body)
-    test_response_json['workflow_runs'][0]['conclusion'] = 'failure'
-    stub_github_test_request(JSON.generate(test_response_json))
-    manager.update(compatible_plugin)
+  it "sets a failing status and message when tests are failing" do
+    PluginManager::Plugin::Status.update(compatible_plugin, git, compatible_status)
+
+    set_workflow(0, 'conclusion', 'failure')
+    set_test_check(2, 'conclusion', 'failure')
+    stub_github_annotations_request(test_annotations_response_body)
+
+    PluginManager::StatusHandler.any_instance.expects(:perform).with(
+      PluginManager::Plugin::Status.statuses[:compatible],
+      PluginManager::Plugin::Status.statuses[:tests_failing],
+      {
+        message: I18n.t("plugin_manager.test.failed_with_message",
+          test_name: "ci / frontend_tests",
+          message: "QUnit Test Failure: Acceptance: Field | Fields: Text, Process completed with exit code 1."
+        )
+      }
+    ).returns(true)
 
-    status = PluginManager::Plugin::Status.get(compatible_plugin, plugin_branch, discourse_branch)
+    subject.update(compatible_plugin)
     expect(status.test_status).to eq(described_class.status[:failing])
   end
 end

spec/fixtures/github/annotations.json

@@ -0,0 +1,26 @@
+[
+  {
+    "path": ".github",
+    "blob_href": "https://github.com/paviliondev/discourse-compatible-plugin/blob/d2fbe349ee9f86d05fdcdbf20e48fc6edd450217/.github",
+    "start_line": 846,
+    "start_column": null,
+    "end_line": 846,
+    "end_column": null,
+    "annotation_level": "failure",
+    "title": "",
+    "message": "QUnit Test Failure: Acceptance: Field | Fields: Text",
+    "raw_details": ""
+  },
+  {
+    "path": ".github",
+    "blob_href": "https://github.com/paviliondev/discourse-compatible-plugin/blob/d2fbe349ee9f86d05fdcdbf20e48fc6edd450217/.github",
+    "start_line": 894,
+    "start_column": null,
+    "end_line": 894,
+    "end_column": null,
+    "annotation_level": "failure",
+    "title": "",
+    "message": "Process completed with exit code 1.",
+    "raw_details": ""
+  }
+]