[ARVADOS] updated: 03d89302f14a4a1c448b32d62679bab14e23fa23

git at public.curoverse.com git at public.curoverse.com
Mon Apr 27 17:53:28 EDT 2015


Summary of changes:
 .../app/controllers/arvados/v1/jobs_controller.rb  |  27 +++++-
 services/api/test/fixtures/jobs.yml                |  35 ++++++-
 services/api/test/fixtures/links.yml               |  30 ++++++
 services/api/test/fixtures/repositories.yml        |   7 ++
 .../arvados/v1/job_reuse_controller_test.rb        | 108 +++++++++++++++------
 services/api/test/test.git.tar                     | Bin 194560 -> 256000 bytes
 services/api/test/unit/collection_test.rb          |   7 ++
 services/api/test/unit/commit_test.rb              |  60 ++++++++++++
 services/api/test/unit/job_test.rb                 |  10 ++
 9 files changed, 249 insertions(+), 35 deletions(-)

       via  03d89302f14a4a1c448b32d62679bab14e23fa23 (commit)
       via  68dfd41d1d5038f7e0a139779ef6ce1c4a7a8351 (commit)
       via  5b0d540ad315ad390831ad58f52b02e86686524f (commit)
       via  4b17e78749b44772e898619e11aef663810ac6ed (commit)
       via  d054beda9bc86bc6cd33a127f4d9e40d04f1620d (commit)
       via  9bfd0d772abcd643e92fed127112e0ed91c00e65 (commit)
      from  97403e08475b328115373a2c6a23e82116199aad (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 03d89302f14a4a1c448b32d62679bab14e23fa23
Merge: 97403e0 68dfd41
Author: Brett Smith <brett at curoverse.com>
Date:   Mon Apr 27 17:52:35 2015 -0400

    Merge branch '5490-crunch-tighten-job-reuse-wip'
    
    Closes #3341, #5388, #5490, #5688, #5784.


commit 68dfd41d1d5038f7e0a139779ef6ce1c4a7a8351
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Apr 21 16:48:42 2015 -0400

    5490: Require exact matches in API server's default job reuse filters.
    
    The previous default filters would reuse jobs that used any Docker
    image with the matching name, and/or Arvados SDK version since the
    named commit.  User feedback indicates this is surprising, and they
    would prefer to have behavior more like the handling around
    script_version: only reuse jobs that have the latest version of a
    symbolic name, or the exact specified hash.

diff --git a/services/api/app/controllers/arvados/v1/jobs_controller.rb b/services/api/app/controllers/arvados/v1/jobs_controller.rb
index d12f6c1..f1ef2d8 100644
--- a/services/api/app/controllers/arvados/v1/jobs_controller.rb
+++ b/services/api/app/controllers/arvados/v1/jobs_controller.rb
@@ -33,20 +33,27 @@ class Arvados::V1::JobsController < ApplicationController
         @filters =
           [["repository", "=", resource_attrs[:repository]],
            ["script", "=", resource_attrs[:script]],
-           ["script_version", "in git",
-            params[:minimum_script_version] || resource_attrs[:script_version]],
            ["script_version", "not in git", params[:exclude_script_versions]],
           ].reject { |filter| filter.last.nil? or filter.last.empty? }
+        if !params[:minimum_script_version].blank?
+          @filters << ["script_version", "in git",
+                       params[:minimum_script_version]]
+        else
+          add_default_git_filter("script_version", resource_attrs[:repository],
+                                 resource_attrs[:script_version])
+        end
         if image_search = resource_attrs[:runtime_constraints].andand["docker_image"]
           if image_tag = resource_attrs[:runtime_constraints]["docker_image_tag"]
             image_search += ":#{image_tag}"
           end
-          @filters.append(["docker_image_locator", "in docker", image_search])
+          image_locator = Collection.
+            for_latest_docker_image(image_search).andand.portable_data_hash
         else
-          @filters.append(["docker_image_locator", "=", nil])
+          image_locator = nil
         end
+        @filters << ["docker_image_locator", "=", image_locator]
         if sdk_version = resource_attrs[:runtime_constraints].andand["arvados_sdk_version"]
-          @filters.append(["arvados_sdk_version", "in git", sdk_version])
+          add_default_git_filter("arvados_sdk_version", "arvados", sdk_version)
         end
         begin
           load_job_specific_filters
@@ -199,6 +206,16 @@ class Arvados::V1::JobsController < ApplicationController
 
   protected
 
+  def add_default_git_filter(attr_name, repo_name, refspec)
+    # Add a filter to @filters for `attr_name` = the latest commit available
+    # in `repo_name` at `refspec`.  No filter is added if refspec can't be
+    # resolved.
+    commits = Commit.find_commit_range(repo_name, nil, refspec, nil)
+    if commit_hash = commits.first
+      @filters << [attr_name, "=", commit_hash]
+    end
+  end
+
   def load_job_specific_filters
     # Convert Job-specific @filters entries into general SQL filters.
     script_info = {"repository" => nil, "script" => nil}
diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index d74ee2e..8a4c345 100644
--- a/services/api/test/fixtures/jobs.yml
+++ b/services/api/test/fixtures/jobs.yml
@@ -209,6 +209,23 @@ previous_docker_job_run:
   docker_image_locator: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   state: Complete
 
+previous_ancient_docker_image_job_run:
+  uuid: zzzzz-8i9sb-t3b460aolxxuldl
+  created_at: <%= 144.minute.ago.to_s(:db) %>
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: active/foo
+  script: hash
+  script_version: 4fe459abe02d9b365932b8f5dc419439ab4e2577
+  script_parameters:
+    input: fa7aeb5140e2848d39b416daeef4ffc5+45
+    an_integer: "2"
+  runtime_constraints:
+    docker_image: arvados/apitestfixture
+  success: true
+  output: ea10d51bcf88862dbcc36eb292017dfd+45
+  docker_image_locator: b519d9cb706a29fc7ea24dbea2f05851+93
+  state: Complete
+
 previous_job_run_with_arvados_sdk_version:
   uuid: zzzzz-8i9sb-eoo0321or2dw2jg
   created_at: <%= 14.minute.ago.to_s(:db) %>
@@ -242,6 +259,20 @@ previous_job_run_no_output:
   output: ~
   state: Complete
 
+previous_job_run_superseded_by_hash_branch:
+  # This supplied_script_version is a branch name with later commits.
+  uuid: zzzzz-8i9sb-aeviezu5dahph3e
+  created_at: <%= 15.minute.ago.to_s(:db) %>
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  repository: active/shabranchnames
+  script: testscript
+  script_version: 7387838c69a21827834586cc42b467ff6c63293b
+  supplied_script_version: 738783
+  script_parameters: {}
+  success: true
+  output: d41d8cd98f00b204e9800998ecf8427e+0
+  state: Complete
+
 nondeterminisic_job_run:
   uuid: zzzzz-8i9sb-cjs4pklxxjykyyy
   created_at: <%= 14.minute.ago.to_s(:db) %>
diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index 7a59518..42ecad3 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -643,6 +643,21 @@ ancient_docker_image_collection_hash:
   properties:
     image_timestamp: "2010-06-10T14:30:00.184019565Z"
 
+ancient_docker_image_collection_tag:
+  uuid: zzzzz-o0j2j-dockercolltagzz
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-12 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-12 14:30:00.184019565 Z
+  updated_at: 2014-06-12 14:30:00.183829316 Z
+  link_class: docker_image_repo+tag
+  name: arvados/apitestfixture:latest
+  tail_uuid: ~
+  head_uuid: zzzzz-4zz18-t68oksiu9m80s4y
+  properties:
+    image_timestamp: "2010-06-10T14:30:00.184019565Z"
+
 docker_image_tag_like_hash:
   uuid: zzzzz-o0j2j-dockerhashtagaa
   owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 8c23118..64d5591 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -323,6 +323,11 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                      new_job['script_version'])
   end
 
+  test "cannot reuse job when hash-like branch includes newer commit" do
+    check_new_job_created_from({job: {script_version: "738783"}},
+                               :previous_job_run_superseded_by_hash_branch)
+  end
+
   BASE_FILTERS = {
     'repository' => ['=', 'active/foo'],
     'script' => ['=', 'hash'],
@@ -510,6 +515,14 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_not_equal(jobs(:previous_docker_job_run).uuid, new_job.uuid)
   end
 
+  test "don't reuse job using older Docker image of same name" do
+    jobspec = {runtime_constraints: {
+        docker_image: "arvados/apitestfixture",
+      }}
+    check_new_job_created_from({job: jobspec},
+                               :previous_ancient_docker_image_job_run)
+  end
+
   test "reuse job with Docker image that has hash name" do
     jobspec = {runtime_constraints: {
         docker_image: "a" * 64,
@@ -694,7 +707,7 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
            "bad refspec not mentioned in error message")
   end
 
-  test "can't reuse job with older Arvados SDK version" do
+  test "don't reuse job with older Arvados SDK version specified by branch" do
     jobspec = {runtime_constraints: {
         arvados_sdk_version: "master",
       }}
@@ -702,6 +715,22 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                                :previous_job_run_with_arvados_sdk_version)
   end
 
+  test "don't reuse job with older Arvados SDK version specified by commit" do
+    jobspec = {runtime_constraints: {
+        arvados_sdk_version: "ca68b24e51992e790f29df5cc4bc54ce1da4a1c2",
+      }}
+    check_new_job_created_from({job: jobspec},
+                               :previous_job_run_with_arvados_sdk_version)
+  end
+
+  test "don't reuse job with newer Arvados SDK version specified by commit" do
+    jobspec = {runtime_constraints: {
+        arvados_sdk_version: "436637c87a1d2bdbf4b624008304064b6cf0e30c",
+      }}
+    check_new_job_created_from({job: jobspec},
+                               :previous_job_run_with_arvados_sdk_version)
+  end
+
   test "reuse job from arvados_sdk_version git filters" do
     prev_job = jobs(:previous_job_run_with_arvados_sdk_version)
     filters_hash = BASE_FILTERS.

commit 5b0d540ad315ad390831ad58f52b02e86686524f
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Apr 21 11:35:01 2015 -0400

    5490: Test API server finds Docker images with hash names.
    
    This is in the same spirit as the previous, analogous commit for git
    branches.

diff --git a/services/api/test/fixtures/links.yml b/services/api/test/fixtures/links.yml
index b8856ef..7a59518 100644
--- a/services/api/test/fixtures/links.yml
+++ b/services/api/test/fixtures/links.yml
@@ -643,6 +643,21 @@ ancient_docker_image_collection_hash:
   properties:
     image_timestamp: "2010-06-10T14:30:00.184019565Z"
 
+docker_image_tag_like_hash:
+  uuid: zzzzz-o0j2j-dockerhashtagaa
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz
+  created_at: 2014-06-11 14:30:00.184389725 Z
+  modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr
+  modified_by_user_uuid: zzzzz-tpzed-000000000000000
+  modified_at: 2014-06-11 14:30:00.184019565 Z
+  updated_at: 2014-06-11 14:30:00.183829316 Z
+  link_class: docker_image_repo+tag
+  name: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa:latest
+  tail_uuid: ~
+  head_uuid: zzzzz-4zz18-1v45jub259sjjgb
+  properties:
+    image_timestamp: "2014-06-10T14:30:00.184019565Z"
+
 job_reader_can_read_previous_job_run:
   # Permission link giving job_reader permission
   # to read previous_job_run
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 3fd56c3..8c23118 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -510,6 +510,13 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
     assert_not_equal(jobs(:previous_docker_job_run).uuid, new_job.uuid)
   end
 
+  test "reuse job with Docker image that has hash name" do
+    jobspec = {runtime_constraints: {
+        docker_image: "a" * 64,
+      }}
+    check_job_reused_from(jobspec, :previous_docker_job_run)
+  end
+
   ["repository", "script"].each do |skip_key|
     test "missing #{skip_key} filter raises an error" do
       filters = filters_from_hash(BASE_FILTERS.reject { |k| k == skip_key })
diff --git a/services/api/test/unit/collection_test.rb b/services/api/test/unit/collection_test.rb
index d8b8365..93f472a 100644
--- a/services/api/test/unit/collection_test.rb
+++ b/services/api/test/unit/collection_test.rb
@@ -253,4 +253,11 @@ class CollectionTest < ActiveSupport::TestCase
       assert c.valid?
     end
   end
+
+  test "find_all_for_docker_image resolves names that look like hashes" do
+    coll_list = Collection.
+      find_all_for_docker_image('a' * 64, nil, [users(:active)])
+    coll_uuids = coll_list.map(&:uuid)
+    assert_includes(coll_uuids, collections(:docker_image).uuid)
+  end
 end

commit 4b17e78749b44772e898619e11aef663810ac6ed
Author: Brett Smith <brett at curoverse.com>
Date:   Tue Apr 21 11:23:19 2015 -0400

    5490: Test API server handling of git hash-like branch names.
    
    If you have a branch that looks like a commit hash, different git subcommands do different things.  After extended discussion on IRC, we've decided we'd like the rules to be:
    
    * If a 40-character hex string references a commit, resolve it to that
      commit.
    * In other cases of ambiguity, prefer branch names over short commit
      hashes.
    
    Fortunately, this corresponds to the behavior of `git rev-list` and
    our existing implementation.  This commit codifies our desired
    behavior with tests.

diff --git a/services/api/test/fixtures/repositories.yml b/services/api/test/fixtures/repositories.yml
index ab2e360..a5aac11 100644
--- a/services/api/test/fixtures/repositories.yml
+++ b/services/api/test/fixtures/repositories.yml
@@ -39,3 +39,10 @@ repository4:
   name: admin/foo4
   created_at: 2015-01-01T00:00:00.123456Z
   modified_at: 2015-01-01T00:00:00.123456Z
+
+has_branch_with_commit_hash_name:
+  uuid: zzzzz-s0uqq-382brsig8rp3668
+  owner_uuid: zzzzz-tpzed-xurymjxw79nv3jz # active user
+  name: active/shabranchnames
+  created_at: 2015-01-01T00:00:00.123456Z
+  modified_at: 2015-01-01T00:00:00.123456Z
diff --git a/services/api/test/test.git.tar b/services/api/test/test.git.tar
index 9b8e4d5..faa0d65 100644
Binary files a/services/api/test/test.git.tar and b/services/api/test/test.git.tar differ
diff --git a/services/api/test/unit/commit_test.rb b/services/api/test/unit/commit_test.rb
index 67068ee..b57c23b 100644
--- a/services/api/test/unit/commit_test.rb
+++ b/services/api/test/unit/commit_test.rb
@@ -75,6 +75,66 @@ class CommitTest < ActiveSupport::TestCase
     assert $?.success?
   end
 
+  # In active/shabranchnames, "7387838c69a21827834586cc42b467ff6c63293b" is
+  # both a commit hash, and the name of a branch that begins from that same
+  # commit.
+  COMMIT_BRANCH_NAME = "7387838c69a21827834586cc42b467ff6c63293b"
+  # A commit that appears in the branch after 7387838c.
+  COMMIT_BRANCH_COMMIT_2 = "abec49829bf1758413509b7ffcab32a771b71e81"
+  # "738783" is another branch that starts from the above commit.
+  SHORT_COMMIT_BRANCH_NAME = COMMIT_BRANCH_NAME[0, 6]
+  # A commit that appears in branch 738783 after 7387838c.
+  SHORT_BRANCH_COMMIT_2 = "77e1a93093663705a63bb4d505698047e109dedd"
+
+  test "find_commit_range min_version prefers commits over branch names" do
+    assert_equal([COMMIT_BRANCH_NAME],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          COMMIT_BRANCH_NAME, nil, nil))
+  end
+
+  test "find_commit_range max_version prefers commits over branch names" do
+    assert_equal([COMMIT_BRANCH_NAME],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          nil, COMMIT_BRANCH_NAME, nil))
+  end
+
+  test "find_commit_range min_version with short branch name" do
+    assert_equal([SHORT_BRANCH_COMMIT_2],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          SHORT_COMMIT_BRANCH_NAME, nil, nil))
+  end
+
+  test "find_commit_range max_version with short branch name" do
+    assert_equal([SHORT_BRANCH_COMMIT_2],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          nil, SHORT_COMMIT_BRANCH_NAME, nil))
+  end
+
+  test "find_commit_range min_version with disambiguated branch name" do
+    assert_equal([COMMIT_BRANCH_COMMIT_2],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          "heads/#{COMMIT_BRANCH_NAME}",
+                                          nil, nil))
+  end
+
+  test "find_commit_range max_version with disambiguated branch name" do
+    assert_equal([COMMIT_BRANCH_COMMIT_2],
+                 Commit.find_commit_range("active/shabranchnames", nil,
+                                          "heads/#{COMMIT_BRANCH_NAME}", nil))
+  end
+
+  test "find_commit_range min_version with unambiguous short name" do
+    assert_equal([COMMIT_BRANCH_NAME],
+                 Commit.find_commit_range("active/shabranchnames",
+                                          COMMIT_BRANCH_NAME[0..-2], nil, nil))
+  end
+
+  test "find_commit_range max_version with unambiguous short name" do
+    assert_equal([COMMIT_BRANCH_NAME],
+                 Commit.find_commit_range("active/shabranchnames", nil,
+                                          COMMIT_BRANCH_NAME[0..-2], nil))
+  end
+
   test "find_commit_range laundry list" do
     authorize_with :active
 
diff --git a/services/api/test/unit/job_test.rb b/services/api/test/unit/job_test.rb
index 3cffbb9..6414889 100644
--- a/services/api/test/unit/job_test.rb
+++ b/services/api/test/unit/job_test.rb
@@ -78,6 +78,16 @@ class JobTest < ActiveSupport::TestCase
     assert(job.invalid?, "Job with bad Docker tag valid")
   end
 
+  test "create a job with a disambiguated script_version branch name" do
+    job = Job.
+      new(script: "testscript",
+          script_version: "heads/7387838c69a21827834586cc42b467ff6c63293b",
+          repository: "active/shabranchnames",
+          script_parameters: {})
+    assert(job.save)
+    assert_equal("abec49829bf1758413509b7ffcab32a771b71e81", job.script_version)
+  end
+
   test "locate a Docker image with a partial hash" do
     image_hash = links(:docker_image_collection_hash).name[0..24]
     job = Job.new job_attrs(runtime_constraints:

commit d054beda9bc86bc6cd33a127f4d9e40d04f1620d
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Apr 16 17:33:22 2015 -0400

    5490: Clean up API server previous job fixtures.
    
    * Every fixture that specified an arvados_sdk_version constraint
      should also specify a docker_image constraint, to better match
      reality.
    
    * Use the same Docker image name in link and previous job fixture
      constraints.

diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml
index aa16134..d74ee2e 100644
--- a/services/api/test/fixtures/jobs.yml
+++ b/services/api/test/fixtures/jobs.yml
@@ -203,7 +203,7 @@ previous_docker_job_run:
     input: fa7aeb5140e2848d39b416daeef4ffc5+45
     an_integer: "1"
   runtime_constraints:
-    docker_image: arvados/test
+    docker_image: arvados/apitestfixture
   success: true
   output: ea10d51bcf88862dbcc36eb292017dfd+45
   docker_image_locator: fa3c1a9cb6783f85f2ecda037e07b8c3+167
@@ -221,7 +221,9 @@ previous_job_run_with_arvados_sdk_version:
     an_integer: "1"
   runtime_constraints:
     arvados_sdk_version: commit2
+    docker_image: arvados/apitestfixture
   arvados_sdk_version: 00634b2b8a492d6f121e3cf1d6587b821136a9a7
+  docker_image_locator: fa3c1a9cb6783f85f2ecda037e07b8c3+167
   success: true
   output: ea10d51bcf88862dbcc36eb292017dfd+45
   state: Complete
diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 6b68e80..3fd56c3 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -696,14 +696,15 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
   end
 
   test "reuse job from arvados_sdk_version git filters" do
+    prev_job = jobs(:previous_job_run_with_arvados_sdk_version)
     filters_hash = BASE_FILTERS.
-      merge("arvados_sdk_version" => ["in git", "commit2"])
+      merge("arvados_sdk_version" => ["in git", "commit2"],
+            "docker_image_locator" => ["=", prev_job.docker_image_locator])
     filters_hash.delete("script_version")
     params = create_job_params(filters: filters_from_hash(filters_hash))
     post(:create, params)
     assert_response :success
-    assert_equal(jobs(:previous_job_run_with_arvados_sdk_version).uuid,
-                 assigns(:object).uuid)
+    assert_equal(prev_job.uuid, assigns(:object).uuid)
   end
 
   test "create new job because of arvados_sdk_version 'not in git' filters" do

commit 9bfd0d772abcd643e92fed127112e0ed91c00e65
Author: Brett Smith <brett at curoverse.com>
Date:   Thu Apr 16 17:32:54 2015 -0400

    5490: Refactor API server job_reuse_controller_test.
    
    Teach the convenience methods to start from any fixture job as a basis
    for submitting new ones and comparing.

diff --git a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
index 1dd620a..6b68e80 100644
--- a/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/job_reuse_controller_test.rb
@@ -599,35 +599,52 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
                     jobs(:previous_docker_job_run).uuid)
   end
 
-  def create_foo_hash_job_params(params)
+  JOB_SUBMIT_KEYS = [:script, :script_parameters, :script_version, :repository]
+  DEFAULT_START_JOB = :previous_job_run
+
+  def create_job_params(params, start_from=DEFAULT_START_JOB)
     if not params.has_key?(:find_or_create)
       params[:find_or_create] = true
     end
     job_attrs = params.delete(:job) || {}
-    params[:job] = {
-      script: "hash",
-      script_version: "4fe459abe02d9b365932b8f5dc419439ab4e2577",
-      repository: "active/foo",
-      script_parameters: {
-        input: 'fa7aeb5140e2848d39b416daeef4ffc5+45',
-        an_integer: '1',
-      },
-    }.merge(job_attrs)
+    start_job = jobs(start_from)
+    params[:job] = Hash[JOB_SUBMIT_KEYS.map do |key|
+                          [key, start_job.send(key)]
+                        end]
+    params[:job][:runtime_constraints] =
+      job_attrs.delete(:runtime_constraints) || {}
+    { arvados_sdk_version: :arvados_sdk_version,
+      docker_image_locator: :docker_image }.each do |method, constraint_key|
+      if constraint_value = start_job.send(method)
+        params[:job][:runtime_constraints][constraint_key] ||= constraint_value
+      end
+    end
+    params[:job].merge!(job_attrs)
     params
   end
 
-  def check_new_job_created_from(params)
-    start_time = Time.now
-    post(:create, create_foo_hash_job_params(params))
+  def create_job_from(params, start_from)
+    post(:create, create_job_params(params, start_from))
     assert_response :success
     new_job = assigns(:object)
     assert_not_nil new_job
+    new_job
+  end
+
+  def check_new_job_created_from(params, start_from=DEFAULT_START_JOB)
+    start_time = Time.now
+    new_job = create_job_from(params, start_from)
     assert_operator(start_time, :<=, new_job.created_at)
     new_job
   end
 
-  def check_errors_from(params)
-    post(:create, create_foo_hash_job_params(params))
+  def check_job_reused_from(params, start_from)
+    new_job = create_job_from(params, start_from)
+    assert_equal(jobs(start_from).uuid, new_job.uuid)
+  end
+
+  def check_errors_from(params, start_from=DEFAULT_START_JOB)
+    post(:create, create_job_params(params, start_from))
     assert_includes(405..499, @response.code.to_i)
     errors = json_response.fetch("errors", [])
     assert(errors.any?, "no errors assigned from #{params}")
@@ -671,22 +688,18 @@ class Arvados::V1::JobReuseControllerTest < ActionController::TestCase
   end
 
   test "can't reuse job with older Arvados SDK version" do
-    params = {
-      script_version: "31ce37fe365b3dc204300a3e4c396ad333ed0556",
-      runtime_constraints: {
-        "arvados_sdk_version" => "master",
-        "docker_image" => links(:docker_image_collection_tag).name,
-      },
-    }
-    check_new_job_created_from(job: params)
+    jobspec = {runtime_constraints: {
+        arvados_sdk_version: "master",
+      }}
+    check_new_job_created_from({job: jobspec},
+                               :previous_job_run_with_arvados_sdk_version)
   end
 
   test "reuse job from arvados_sdk_version git filters" do
     filters_hash = BASE_FILTERS.
       merge("arvados_sdk_version" => ["in git", "commit2"])
     filters_hash.delete("script_version")
-    params = create_foo_hash_job_params(filters:
-                                        filters_from_hash(filters_hash))
+    params = create_job_params(filters: filters_from_hash(filters_hash))
     post(:create, params)
     assert_response :success
     assert_equal(jobs(:previous_job_run_with_arvados_sdk_version).uuid,

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list