Fix pipeline instance state tests.

diff --git a/services/api/app/models/pipeline_instance.rb b/services/api/app/models/pipeline_instance.rb
index 2be4a066fcf245a668093542d367887860e4d9b3..e5bd46448fbf8679bfef727bc4acf17e3dae99a6 100644 (file)
--- a/services/api/app/models/pipeline_instance.rb
+++ b/services/api/app/models/pipeline_instance.rb
@@ -27,13 +27,16 @@ class PipelineInstance < ArvadosModel
   end
 
   # Supported states for a pipeline instance
-  New = 'New'
-  Ready = 'Ready'
-  RunningOnServer = 'RunningOnServer'
-  RunningOnClient = 'RunningOnClient'
-  Paused = 'Paused'
-  Failed = 'Failed'
-  Complete = 'Complete'
+  States =
+    [
+     (New = 'New'),
+     (Ready = 'Ready'),
+     (RunningOnServer = 'RunningOnServer'),
+     (RunningOnClient = 'RunningOnClient'),
+     (Paused = 'Paused'),
+     (Failed = 'Failed'),
+     (Complete = 'Complete'),
+    ]
 
   def dependencies
     dependency_search(self.components).keys
@@ -172,7 +175,7 @@ class PipelineInstance < ArvadosModel
       end
     elsif 'active'.in? changed_attributes
       if self.active
-        if self.state == New || self.state == Ready || self.state == Paused
+        if self.state.in? [New, Ready, Paused]
           self.state = RunningOnServer
         end
       else
@@ -185,13 +188,24 @@ class PipelineInstance < ArvadosModel
           self.state = New
         end
       end
+    elsif new_record? and self.state.nil?
+      # No state, active, or success given
+      self.state = New
     end
 
-    if 'components'.in? changed_attributes
-      if self.components_look_ready? && (!self.state || self.state == New)
+    if new_record? or 'components'.in? changed_attributes
+      state ||= New
+      if state == New and self.components_look_ready?
         self.state = Ready
       end
     end
+
+    if self.state.in?(States)
+      true
+    else
+      errors.add :state, "'#{state.inspect} must be one of: [#{States.join ', '}]"
+      false
+    end
   end
 
   def set_state_before_save

diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml
index aa353952a940cd5d0aa9d105df2e32ea0558ccaa..b5e1bc1c6bf2e44683b2ff18eccfc7593c8c8818 100644 (file)
--- a/services/api/test/fixtures/pipeline_instances.yml
+++ b/services/api/test/fixtures/pipeline_instances.yml
@@ -1,8 +1,10 @@
 new_pipeline:
+  state: New
   uuid: zzzzz-d1hrv-f4gneyn6br1xize
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
 
 has_component_with_no_script_parameters:
+  state: Ready
   uuid: zzzzz-d1hrv-1xfj6xkicf2muk2
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   components:
@@ -12,6 +14,7 @@ has_component_with_no_script_parameters:
     script_parameters: {}
 
 has_component_with_empty_script_parameters:
+  state: Ready
   uuid: zzzzz-d1hrv-jq16l10gcsnyumo
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
   components:

diff --git a/services/api/test/unit/pipeline_instance_test.rb b/services/api/test/unit/pipeline_instance_test.rb
index 7b618140d14e30594178fb644f84352712410420..0f2c2edad83dc7ff15b561c536c637e040febd6e 100644 (file)
--- a/services/api/test/unit/pipeline_instance_test.rb
+++ b/services/api/test/unit/pipeline_instance_test.rb
@@ -5,9 +5,9 @@ class PipelineInstanceTest < ActiveSupport::TestCase
   test "check active and success for a pipeline in new state" do
     pi = pipeline_instances :new_pipeline
 
-    assert !pi.active, 'expected active to be false for a new pipeline'
-    assert !pi.success, 'expected success to be false for a new pipeline'
-    assert !pi.state, 'expected state to be nil because the fixture had no state specified'
+    assert !pi.active, 'expected active to be false for :new_pipeline'
+    assert !pi.success, 'expected success to be false for :new_pipeline'
+    assert_equal 'New', pi.state, 'expected state to be New for :new_pipeline'
 
     # save the pipeline and expect state to be New
     Thread.current[:user] = users(:admin)
@@ -19,6 +19,18 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     assert !pi.success, 'expected success to be false for a new pipeline'
   end
 
+  test "check active and success for a newly created pipeline" do
+    set_user_from_auth :active
+
+    pi = PipelineInstance.create(state: 'Ready')
+    pi.save
+
+    assert pi.valid?, 'expected newly created empty pipeline to be valid ' + pi.errors.messages.to_s
+    assert !pi.active, 'expected active to be false for a new pipeline'
+    assert !pi.success, 'expected success to be false for a new pipeline'
+    assert_equal 'Ready', pi.state, 'expected state to be Ready for a new empty pipeline'
+  end
+
   test "update attributes for pipeline" do
     Thread.current[:user] = users(:admin)
 
@@ -58,7 +70,7 @@ class PipelineInstanceTest < ActiveSupport::TestCase
     assert !pi.success, 'expected success to be false for a new pipeline'
 
     pi.active = true
-    pi.save
+    assert_equal true, pi.save, 'expected pipeline instance to save, but ' + pi.errors.messages.to_s
     pi = PipelineInstance.find_by_uuid 'zzzzz-d1hrv-f4gneyn6br1xize'
     assert_equal PipelineInstance::RunningOnServer, pi.state, 'expected state to be RunningOnServer after updating active to true'
     assert pi.active, 'expected active to be true after update'
@@ -134,9 +146,9 @@ class PipelineInstanceTest < ActiveSupport::TestCase
 
       Thread.current[:user] = users(:active)
       # Make sure we go through the "active_changed? and active" code:
-      pi.update_attributes active: true
-      pi.update_attributes active: false
-      assert_equal PipelineInstance::Ready, pi.state
+      assert_equal true, pi.update_attributes(active: true), pi.errors.messages
+      assert_equal true, pi.update_attributes(active: false), pi.errors.messages
+      assert_equal PipelineInstance::Paused, pi.state
     end
   end
 end