12840: Differentiate "Cancelled" from "Failed" pipeline instances.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/apps/workbench/app/models/pipeline_instance_work_unit.rb b/apps/workbench/app/models/pipeline_instance_work_unit.rb
index 517dbc7a352a78502117c85662cc1e7318739eb8..de706896562c6f6f27c30eedd8432a32537c2f8b 100644 (file)
--- a/apps/workbench/app/models/pipeline_instance_work_unit.rb
+++ b/apps/workbench/app/models/pipeline_instance_work_unit.rb
@@ -59,4 +59,19 @@ class PipelineInstanceWorkUnit < ProxyWorkUnit
   def template_uuid
     get(:pipeline_template_uuid)
   end
+
+  def state_label
+    # Show "Cancelled" instead of "Failed" if there are no failed
+    # components. #12840
+    if get(:state) != "Failed"
+      return super
+    end
+    get(:components).each do |_, c|
+      jstate = c[:job][:state] rescue nil
+      if jstate == "Failed"
+        return "Failed"
+      end
+    end
+    "Cancelled"
+  end
 end

diff --git a/apps/workbench/app/models/proxy_work_unit.rb b/apps/workbench/app/models/proxy_work_unit.rb
index f570563b751ee66ec074733a548b41e6fc4a98fe..02f6b42fd926eb432bec264c87b8def198a06151 100644 (file)
--- a/apps/workbench/app/models/proxy_work_unit.rb
+++ b/apps/workbench/app/models/proxy_work_unit.rb
@@ -255,7 +255,9 @@ class ProxyWorkUnit < WorkUnit
       if state_label == 'Complete'
         resp << "completed in "
       elsif state_label == 'Failed'
-         resp << "failed after "
+        resp << "failed after "
+      elsif state_label == 'Cancelled'
+        resp << "was cancelled after "
       else
         resp << "has been active for "
       end

diff --git a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
index 1cd9445a5a1212a693ab6f80c64f7b98cd670fd7..60d4c2abc2077e44d3472f6528d6697374829d20 100644 (file)
--- a/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
+++ b/apps/workbench/app/views/pipeline_instances/_show_components_running.html.erb
@@ -34,6 +34,8 @@ SPDX-License-Identifier: AGPL-3.0 %>
       completed in
     <% elsif @object.state == 'Failed' %>
       failed after
+    <% elsif @object.state == 'Cancelled' %>
+      was cancelled after
     <% else %>
       has been active for
     <% end %>

diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb
index f3c0789cb6aee9fc302c14bba6a3c4622bfa8476..47e385a4e27830b4ab5293c9996a933f1496c3f6 100644 (file)
--- a/apps/workbench/test/integration/pipeline_instances_test.rb
+++ b/apps/workbench/test/integration/pipeline_instances_test.rb
@@ -476,30 +476,22 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
       need_selenium 'to parse timestamps correctly across DST boundaries'
       visit page_with_token(user, "/pipeline_instances/#{uuid}")
 
-      assert page.has_text? 'This pipeline started at'
-      page_text = page.text
-
+      regexp = "This pipeline started at (.+?)\\. "
       if run_time
-        match = /This pipeline started at (.*)\. It failed after (.*) at (.*)\. Check the Log/.match page_text
+        regexp += "It failed after (.+?) at (.+?)\\. Check the Log"
       else
-        match = /This pipeline started at (.*). It has been active for(.*)/.match page_text
+        regexp += "It has been active for \\d"
       end
-      assert_not_nil(match, 'Did not find text - This pipeline started at . . . ')
+      assert_match /#{regexp}/, page.text
 
-      start_at = match[1]
-      assert_not_nil(start_at, 'Did not find start_at time')
+      return if !run_time
 
-      start_time = parse_browser_timestamp start_at
-      if run_time
-        finished_at = match[3]
-        assert_not_nil(finished_at, 'Did not find finished_at time')
-        finished_time = parse_browser_timestamp finished_at
-        assert_equal(run_time, finished_time-start_time,
-          "Time difference did not match for start_at #{start_at}, finished_at #{finished_at}, ran_for #{match[2]}")
-      else
-        match = /\d(.*)/.match match[2]
-        assert_not_nil match, 'Did not find expected match for running component'
-      end
+      # match again to capture (.*)
+      _, started, duration, finished = *(/#{regexp}/.match(page.text))
+      assert_equal(
+        run_time,
+        parse_browser_timestamp(finished) - parse_browser_timestamp(started),
+        "expected: #{run_time}, got: started #{started}, finished #{finished}, duration #{duration}")
     end
   end
 

diff --git a/apps/workbench/test/unit/work_unit_test.rb b/apps/workbench/test/unit/work_unit_test.rb
index 5cf9499aad5201cb9e61074fc3294c49bc981785..c5a6d8f4775f2e908653a1444e9954ce8014d796 100644 (file)
--- a/apps/workbench/test/unit/work_unit_test.rb
+++ b/apps/workbench/test/unit/work_unit_test.rb
@@ -17,6 +17,7 @@ class WorkUnitTest < ActiveSupport::TestCase
     [PipelineInstance, 'pipeline_in_running_state', nil, 1, "Running", nil, 0.0],
     [PipelineInstance, 'has_component_with_completed_jobs', nil, 3, "Complete", true, 1.0],
     [PipelineInstance, 'pipeline_with_tagged_collection_input', "pwu", 1, "Ready", nil, 0.0],
+    [PipelineInstance, 'failed_pipeline_with_two_jobs', nil, 2, "Cancelled", false, 0.0],
     [Container, 'requester', 'cwu', 1, "Complete", true, 1.0],
     [ContainerRequest, 'cr_for_requester', 'cwu', 1, "Complete", true, 1.0],
     [ContainerRequest, 'queued', 'cwu', 0, "Queued", nil, 0.0],   # priority 1

diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml
index 5267a0b2e196fb055d47b03c6d6f8b86c9245d64..013f03c47e55b154da228e20a934bee106239676 100644 (file)
--- a/services/api/test/fixtures/pipeline_instances.yml
+++ b/services/api/test/fixtures/pipeline_instances.yml
@@ -486,6 +486,8 @@ pipeline_<%=i%>_of_10:
           required: true
           dataclass: Collection
           title: foo instance input
+      job:
+        state: Failed
 <% end %>
 
 # pipelines in project_with_2_pipelines_and_60_crs