17010: Fix tests. Tests check that --enable-reuse is set
authorPeter Amstutz <peter.amstutz@curii.com>
Thu, 22 Oct 2020 02:16:26 +0000 (22:16 -0400)
committerPeter Amstutz <peter.amstutz@curii.com>
Fri, 23 Oct 2020 01:02:16 +0000 (21:02 -0400)
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <peter.amstutz@curii.com>

apps/workbench/app/controllers/container_requests_controller.rb
apps/workbench/test/controllers/container_requests_controller_test.rb
apps/workbench/test/integration/work_units_test.rb

index c217eb977a2350def6c5a6ab9550d89d6b79b37a..62ec3e15073802f4f184ddbbb258263887d3b77b 100644 (file)
@@ -127,14 +127,12 @@ class ContainerRequestsController < ApplicationController
         @updates[:reuse_steps] = false
       end
       @updates[:command] ||= @object.command
+      @updates[:command] -= ["--disable-reuse", "--enable-reuse"]
       if @updates[:reuse_steps]
-        @updates[:command] = @updates[:command] - ["--disable-reuse"]
-        @updates[:command].insert(1, '--enable-reuse')
+        @updates[:command].insert(1, "--enable-reuse")
       else
-        @updates[:command] -= @updates[:command] - ["--enable-reuse"]
-        @updates[:command].insert(1, '--disable-reuse')
+        @updates[:command].insert(1, "--disable-reuse")
       end
-
       @updates.delete(:reuse_steps)
     end
 
@@ -167,26 +165,31 @@ class ContainerRequestsController < ApplicationController
         if arg.start_with? "--project-uuid="
           command[i] = "--project-uuid=#{@object.owner_uuid}"
         end
-        if arg == "--disable-reuse"
-          command[i] = "--enable-reuse"
-        end
       end
+      command -= ["--disable-reuse", "--enable-reuse"]
+      command.insert(1, '--enable-reuse')
+    end
+
+    if params[:use_existing] == "false"
+      params[:use_existing] = false
+    elsif params[:use_existing] == "true"
+      params[:use_existing] = true
     end
 
-    # By default the copied CR won't be reusing containers, unless use_existing=true
-    # param is passed.
-    if params[:use_existing]
-      @object.use_existing = true
+    if params[:use_existing] || params[:use_existing].nil?
+      # If nil, reuse workflow steps but not the workflow runner.
+      @object.use_existing = (params[:use_existing] ? true : false)
+
       # Pass the correct argument to arvados-cwl-runner command.
       if command[0] == 'arvados-cwl-runner'
-        command = src.command - ['--disable-reuse']
+        command -= ['--disable-reuse']
         command.insert(1, '--enable-reuse')
       end
     else
       @object.use_existing = false
       # Pass the correct argument to arvados-cwl-runner command.
-      if src.command[0] == 'arvados-cwl-runner'
-        command = src.command - ['--enable-reuse']
+      if command[0] == 'arvados-cwl-runner'
+        command -= ['--enable-reuse']
         command.insert(1, '--disable-reuse')
       end
     end
index c4a723930da433c0240423ef1f416a2b96720524..73d357f3a60f6a9da27db76a452a5ded6b0e3bd8 100644 (file)
@@ -60,17 +60,19 @@ class ContainerRequestsControllerTest < ActionController::TestCase
   end
 
   [
-    ['completed', false, false],
-    ['completed', true, false],
+    ['completed',       false, false],
+    ['completed',        true, false],
+    ['completed',         nil, false],
     ['completed-older', false, true],
-    ['completed-older', true, true],
+    ['completed-older',  true, true],
+    ['completed-older',   nil, true],
   ].each do |cr_fixture, reuse_enabled, uses_acr|
-    test "container request #{uses_acr ? '' : 'not'} using arvados-cwl-runner copy #{reuse_enabled ? 'with' : 'without'} reuse enabled" do
+    test "container request #{uses_acr ? '' : 'not'} using arvados-cwl-runner copy #{reuse_enabled.nil? ? 'nil' : (reuse_enabled ? 'with' : 'without')} reuse enabled" do
       completed_cr = api_fixture('container_requests')[cr_fixture]
       # Set up post request params
       copy_params = {id: completed_cr['uuid']}
-      if reuse_enabled
-        copy_params.merge!({use_existing: true})
+      if !reuse_enabled.nil?
+        copy_params.merge!({use_existing: reuse_enabled})
       end
       post(:copy, params: copy_params, session: session_for(:active))
       assert_response 302
@@ -87,12 +89,11 @@ class ContainerRequestsControllerTest < ActionController::TestCase
       # If the CR's command is arvados-cwl-runner, the appropriate flag should
       # be passed to it
       if uses_acr
-        if reuse_enabled
-          # arvados-cwl-runner's default behavior is to enable reuse
-          assert_includes copied_cr['command'], 'arvados-cwl-runner'
+        assert_equal copied_cr['command'][0], 'arvados-cwl-runner'
+        if reuse_enabled.nil? || reuse_enabled
+          assert_includes copied_cr['command'], '--enable-reuse'
           assert_not_includes copied_cr['command'], '--disable-reuse'
         else
-          assert_includes copied_cr['command'], 'arvados-cwl-runner'
           assert_includes copied_cr['command'], '--disable-reuse'
           assert_not_includes copied_cr['command'], '--enable-reuse'
         end
index 43740f192d641772e3515a9ddc2ed92d68b3f5ff..4f2ebbc554d624440cd4dc5251667c7c5ecadfba 100644 (file)
@@ -163,7 +163,9 @@ class WorkUnitsTest < ActionDispatch::IntegrationTest
       assert_text process_txt
       assert_selector 'a', text: template_name
 
-      assert_equal "Set value for ex_string_def", find('div.form-group > div > p.form-control-static > a', text: "hello-testing-123")[:"data-title"]
+      assert_equal "true", find('span[data-name="reuse_steps"]').text
+
+      assert_equal "Set value for ex_string_def", find('div.form-group > div.form-control-static > a', text: "hello-testing-123")[:"data-title"]
 
       page.assert_selector 'a.disabled,button.disabled', text: 'Run'
     end