[ARVADOS] updated: d694a717acb2e577afa396aea140e5284b7f763d

[Git user]

Summary of changes:
 .../controllers/container_requests_controller.rb   |  4 +-
 .../_extra_tab_line_buttons.html.erb               | 11 +++-
 .../container_requests_controller_test.rb          | 71 +++++++++++++---------
 services/api/test/fixtures/container_requests.yml  | 19 ++++++
 4 files changed, 72 insertions(+), 33 deletions(-)

       via  d694a717acb2e577afa396aea140e5284b7f763d (commit)
      from  65da23323c06079612db9285e0ab2bd1ac9ea253 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit d694a717acb2e577afa396aea140e5284b7f763d
Author: Lucas Di Pentima <lucas at curoverse.com>
Date:   Fri Apr 28 16:18:56 2017 -0300

    11185: Addressed several requests:
    * Added '...' to 'Re-run' button's title to indicate it will show a dialog.
    * Corrected comment.
    * Added javascript code so that the 'Cancel' and 'x' buttons on the dialog reset the checkbox.
    * Expanded test cases to cover CRs that both do and don't use arvados-cwl-runner command, with
    and without container reuse.

diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb
index 51ad4b6..0fc8cbd 100644
--- a/apps/workbench/app/controllers/container_requests_controller.rb
+++ b/apps/workbench/app/controllers/container_requests_controller.rb
@@ -64,8 +64,8 @@ class ContainerRequestsController < ApplicationController
 
     @object = ContainerRequest.new
 
-    # By default the copied CR won't be reusing jobs, unless use_existing=true param
-    # is passed.
+    # By default the copied CR won't be reusing containers, unless use_existing=true
+    # param is passed.
     command = src.command
     if params[:use_existing]
       @object.use_existing = true
diff --git a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
index fc810b1..9cf03c8 100644
--- a/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
+++ b/apps/workbench/app/views/container_requests/_extra_tab_line_buttons.html.erb
@@ -1,10 +1,15 @@
 <% if @object.state == 'Final' %>
-  <%= link_to raw('<i class="fa fa-fw fa-play"></i> Re-run'),
+  <%= link_to raw('<i class="fa fa-fw fa-play"></i> Re-run...'),
       "#",
       {class: 'btn btn-sm btn-primary', 'data-toggle' => 'modal',
        'data-target' => '#clone-and-edit-modal-window',
        title: 'This will make a copy and take you there. You can then make any needed changes and run it'}  %>
-<% end %>
+
+<script type="application/javascript">
+  function reset_form() {
+    $('#use_existing').removeAttr('checked');
+  }
+</script>
 
 <div id="clone-and-edit-modal-window" class="modal fade" role="dialog"
      aria-labelledby="myModalLabel" aria-hidden="true">
@@ -35,3 +40,5 @@
     <% end %>
   </div>
 </div>
+
+<% end %>
diff --git a/apps/workbench/test/controllers/container_requests_controller_test.rb b/apps/workbench/test/controllers/container_requests_controller_test.rb
index 7b0f6fc..804263c 100644
--- a/apps/workbench/test/controllers/container_requests_controller_test.rb
+++ b/apps/workbench/test/controllers/container_requests_controller_test.rb
@@ -41,34 +41,47 @@ class ContainerRequestsControllerTest < ActionController::TestCase
    assert_includes @response.body, "action=\"/container_requests/#{uuid}/copy\""
   end
 
-  test "container request copy" do
-    completed_cr = api_fixture('container_requests')['completed']
-    post(:copy,
-         {
-           id: completed_cr['uuid']
-         },
-         session_for(:active))
-    assert_response 302
-    copied_cr = assigns(:object)
-    assert_not_nil copied_cr
-    assert_equal 'Uncommitted', copied_cr[:state]
-    assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
-    assert_equal completed_cr['cmd'], copied_cr['cmd']
-    assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
-    refute copied_cr[:use_existing]
-  end
-
-  test "container request copy with reuse enabled" do
-    completed_cr = api_fixture('container_requests')['completed']
-    post(:copy,
-         {
-           id: completed_cr['uuid'],
-           use_existing: true,
-         },
-         session_for(:active))
-    assert_response 302
-    copied_cr = assigns(:object)
-    assert_not_nil copied_cr
-    assert copied_cr['use_existing']
+  [
+    ['completed', false, false],
+    ['completed', true, false],
+    ['completed-acr', false, true],
+    ['completed-acr', true, 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
+      completed_cr = api_fixture('container_requests')[cr_fixture]
+      copy_params = {id: completed_cr['uuid']}
+      if reuse_enabled
+        copy_params.merge!({use_existing: true})
+      end
+      post(:copy, copy_params, session_for(:active))
+      assert_response 302
+      copied_cr = assigns(:object)
+      assert_not_nil copied_cr
+      assert_equal 'Uncommitted', copied_cr[:state]
+      assert_equal "Copy of #{completed_cr['name']}", copied_cr['name']
+      assert_equal completed_cr['cmd'], copied_cr['cmd']
+      assert_equal completed_cr['runtime_constraints']['ram'], copied_cr['runtime_constraints'][:ram]
+      if reuse_enabled
+        assert copied_cr[:use_existing]
+      else
+        refute copied_cr[:use_existing]
+      end
+      # 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_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
+      else
+        # If no arvados-cwl-runner is being used, the command should be left alone
+        assert_equal completed_cr['command'], copied_cr['command']
+      end
+    end
   end
 end
diff --git a/services/api/test/fixtures/container_requests.yml b/services/api/test/fixtures/container_requests.yml
index 76f59c2..b35829a 100644
--- a/services/api/test/fixtures/container_requests.yml
+++ b/services/api/test/fixtures/container_requests.yml
@@ -115,6 +115,25 @@ completed-older:
     vcpus: 1
     ram: 123
 
+completed-acr:
+  uuid: zzzzz-xvhdp-cr4completedacr
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  name: completed
+  state: Final
+  priority: 1
+  created_at: 2016-01-11 11:11:11.111111111 Z
+  updated_at: 2016-01-11 11:11:11.111111111 Z
+  modified_at: 2016-01-11 11:11:11.111111111 Z
+  modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  container_image: test
+  cwd: test
+  output_path: test
+  command: ["arvados-cwl-runner", "echo", "hello"]
+  container_uuid: zzzzz-dz642-compltcontainr2
+  runtime_constraints:
+    vcpus: 1
+    ram: 123
+
 requester:
   uuid: zzzzz-xvhdp-9zacv3o1xw6sxz5
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz

-----------------------------------------------------------------------


hooks/post-receive
--