3898: add unit test for job state attribute

diff --git a/services/api/app/models/job.rb b/services/api/app/models/job.rb
index a831dd68b7e720bd1ef9d8959badcd79c574f760..fd46a6ff12d5cfa4344e5be1e5f99f4d3c47f522 100644 (file)
--- a/services/api/app/models/job.rb
+++ b/services/api/app/models/job.rb
@@ -263,7 +263,7 @@ class Job < ArvadosModel
           self.cancelled_at = Time.now
         end
         self.running = false
-        self.success = false
+        self.success = nil
       when Failed
         if !self.finished_at
           self.finished_at = Time.now
@@ -278,16 +278,24 @@ class Job < ArvadosModel
         self.success = true
       end
     elsif 'running'.in? changed_attributes
-      self.state = Running
+      if self.running
+        self.state = Running
+        if !self.started_at
+          self.started_at = Time.now
+        end
+      end
     elsif 'success'.in? changed_attributes
-      if success
+      if self.success
         self.state = Complete
       else
         self.state = Failed
       end
+      self.running = false
     elsif 'cancelled_at'.in? changed_attributes
       self.state = Cancelled
+      self.running = false
     end
+    true
   end
 
   def set_state_before_save
@@ -298,13 +306,14 @@ class Job < ArvadosModel
         self.state = Complete
       elsif (!self.success.nil? && !self.success)
         self.state = Failed
-      elsif (self.running && self.success.nil? && !self.cencelled_at)
+      elsif (self.running && self.success.nil? && !self.cancelled_at)
         self.state = Running
-      elsif !self.started_at && !self.cancelled_at && !self.is_locked_by_uuid && self.success.nil?
+      elsif !self.started_at && !self.cancelled_at && !self.is_locked_by_uuid &&
+            self.success.nil? && self.running.nil?
         self.state = Queued
       end
     end
-
+ 
     if self.state.in?(States)
       true
     else

diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 730eb0634a5cc8f18373eb1b9ff219ca072bbeb2..e924cddfc5221039ffadd8d9d16e1854aac01ad5 100644 (file)
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -154,4 +154,60 @@ class JobTest < ActiveSupport::TestCase
       assert_not_empty job.errors, "validation failure did not provide errors"
     end
   end
+
+  [
+    # Each test case is of the following format
+    # Array of parameters where each parameter is of the format:
+    #     attr name to be changed, attr value, (array of array of expectations OR the string "error")
+    [['running', false, [['state', 'Queued']]]],
+    [['state', 'Running', [['running', true], ['started_at', 'not_nil'], ['success', 'nil']]]],
+    [['running', false, [['state', 'Queued']]], ['state', 'Complete', [['success', true]]]],
+    [['running', true, [['state', 'Running']]], ['cancelled_at', Time.now, [['state', 'Cancelled'],['running', false]]]],
+    [['running', true, [['state', 'Running']]], ['state', 'Cancelled', [['running', false],['cancelled_at', 'not_nil']]]],
+    [['running', true, [['state', 'Running']]], ['success', true, [['state', 'Complete'],['running', false]]]],
+    [['running', true, [['state', 'Running']]], ['success', 'false', [['state', 'Failed'],['running', false]]]],
+    [['running', true, [['state', 'Running']]], ['state', 'Complete', [['success', true],['running', false]]]],
+    [['running', true, [['state', 'Running']]], ['state', 'Failed', [['success', false],['running', false]]]],
+    # potential migration cases
+    [['state', nil, [['state', 'Queued']]]],
+    [['state', nil, [['state', 'Queued']]], ['cancelled_at', Time.now, [['state', 'Cancelled']]]],
+    [['running', true, [['state', 'Running'], ['started_at', 'not_nil']]], ['state', nil, [['state', 'Running']]]],
+    # bogus initial status (started_at but not running), to produce error while setting state
+    [['started_at', Time.now, [['state', 'Queued']]], ['state', nil, 'error']],
+  ].each do |parameters|
+    test "verify job status #{parameters}" do
+      job = Job.create! job_attrs
+      assert job.valid?, job.errors.full_messages.to_s
+      assert_equal job.state, 'Queued'
+
+      parameters.each do |parameter|
+        expectations = parameter[2]
+        if expectations.instance_of? Array
+          job[parameter[0]] = parameter[1]
+          job.save!
+          expectations.each do |expectation|
+            if expectation[1] == 'not_nil'
+              assert_not_nil job[expectation[0]]
+            elsif expectation[1] == 'nil'
+              assert_nil job[expectation[0]]
+            else
+              assert_equal expectation[1], job[expectation[0]]
+            end
+          end
+        else # String expectation, looking for error
+          if expectations == 'error'
+            rescued = false
+            begin
+              job[parameter[0]] = parameter[1]
+              job.save!
+            rescue
+              rescued = true
+            end
+            assert rescued, 'Expected error'
+          end
+        end
+      end
+    end
+  end
+
 end