4031: Improve provenance graph tests for readability, use fixtures instead of

hardcoded hashes.  Improve efficiency of matching collections in search_edges.
Remove spurious debug code.

diff --git a/apps/workbench/app/controllers/jobs_controller.rb b/apps/workbench/app/controllers/jobs_controller.rb
index 00ce0a53825fa0228c267cc9365080e62099a630..536518277f1ca86da6256af0887a5be9ee7bc560 100644 (file)
--- a/apps/workbench/app/controllers/jobs_controller.rb
+++ b/apps/workbench/app/controllers/jobs_controller.rb
@@ -24,10 +24,6 @@ class JobsController < ApplicationController
       nodes[c[:portable_data_hash]] = c
     end
 
-    nodes.each do |n|
-      puts "\n#{n.inspect}"
-    end
-
     @svg = ProvenanceHelper::create_provenance_graph nodes, "provenance_svg", {
       :request => request,
       :all_script_parameters => true,

diff --git a/apps/workbench/app/helpers/provenance_helper.rb b/apps/workbench/app/helpers/provenance_helper.rb
index 89d9ee6ceb2ddefebc499aa9f4ba0d5bac570172..e8850d5b47e6e41d10330a58b32baac52219b800 100644 (file)
--- a/apps/workbench/app/helpers/provenance_helper.rb
+++ b/apps/workbench/app/helpers/provenance_helper.rb
@@ -218,7 +218,7 @@ module ProvenanceHelper
 
         label = "#{v[0][:script]}"
 
-        if label == "run-command"
+        if label == "run-command" and v[0][:script_parameters][:command].is_a? Array
           label = v[0][:script_parameters][:command].join(' ')
         end
 
@@ -234,7 +234,7 @@ module ProvenanceHelper
     end
 
     def encode_quotes value
-      value.andand.to_s.gsub("\"", "\\\"").gsub("\n", "\\n")
+      value.to_s.gsub("\"", "\\\"").gsub("\n", "\\n")
     end
   end
 
@@ -301,7 +301,9 @@ edge [fontsize=10,fontname=\"Helvetica,Arial,sans-serif\"];
     svg = svg.sub(/<svg /, "<svg id=\"#{svgId}\" ")
   end
 
-  # returns hash, uuid
+  # yields hash, uuid
+  # Position indicates whether it is a content hash or arvados uuid.
+  # One will hold a value, the other will always be nil.
   def self.find_collections(sp, key=nil, &b)
     case sp
     when ArvadosBase

diff --git a/apps/workbench/test/controllers/collections_controller_test.rb b/apps/workbench/test/controllers/collections_controller_test.rb
index 95f9151e295d084a5897769708e9c6cd1c8145fb..f4d9c4996033b9d37a6685f8e6b9caec168efc38 100644 (file)
--- a/apps/workbench/test/controllers/collections_controller_test.rb
+++ b/apps/workbench/test/controllers/collections_controller_test.rb
@@ -20,7 +20,8 @@ class CollectionsControllerTest < ActionController::TestCase
 
   test 'provenance graph' do
     use_token 'admin'
-    obj = Collection.where(uuid: 'zzzzz-4zz18-uukreo9rbgwsujj').results.first
+
+    obj = find_fixture Collection, "graph_test_collection3"
 
     provenance = obj.provenance.stringify_keys
 
@@ -33,28 +34,26 @@ class CollectionsControllerTest < ActionController::TestCase
                                                            :direction => :bottom_up,
                                                            :combine_jobs => :script_only})
 
-    # hash -> baz file
-    assert /ea10d51bcf88862dbcc36eb292017dfd\+45&#45;&gt;hash_f866587e2de5291fbd38d616d6d33eab/.match(prov_svg)
+    stage1 = find_fixture Job, "graph_stage1"
+    stage3 = find_fixture Job, "graph_stage3"
+    previous_job_run = find_fixture Job, "previous_job_run"
 
-    # hash2 -> baz file
-    assert /ea10d51bcf88862dbcc36eb292017dfd\+45&#45;&gt;hash2_02a085407e751d00b5dc88f1bd5e8247/.match(prov_svg)
+    obj_id = obj.portable_data_hash.gsub('+', '\\\+')
+    stage1_out = stage1.output.gsub('+', '\\\+')
+    stage1_id = "#{stage1.script}_#{Digest::MD5.hexdigest(stage1[:script_parameters].to_json)}"
+    stage3_id = "#{stage3.script}_#{Digest::MD5.hexdigest(stage3[:script_parameters].to_json)}"
 
-    # owned_by_active -> hash
-    assert /hash_f866587e2de5291fbd38d616d6d33eab&#45;&gt;fa7aeb5140e2848d39b416daeef4ffc5\+45/.match(prov_svg)
+    assert /#{obj_id}&#45;&gt;#{stage3_id}/.match(prov_svg)
 
-    # owned_by_active -> hash2
-    assert /hash2_02a085407e751d00b5dc88f1bd5e8247&#45;&gt;fa7aeb5140e2848d39b416daeef4ffc5\+45/.match(prov_svg)
+    assert /#{stage3_id}&#45;&gt;#{stage1_out}/.match(prov_svg)
 
-    # File::open "./tmp/stuff3.svg", "w" do |f|
-    #   f.write "<?xml version=\"1.0\" ?>\n"
-    #   f.write prov_svg
-    # end
+    assert /#{stage1_out}&#45;&gt;#{stage1_id}/.match(prov_svg)
 
   end
 
   test 'used_by graph' do
     use_token 'admin'
-    obj = Collection.where(uuid: 'zzzzz-4zz18-bv31uwvy3neko22').results.first
+    obj = find_fixture Collection, "graph_test_collection1"
 
     used_by = obj.used_by.stringify_keys
 
@@ -64,20 +63,22 @@ class CollectionsControllerTest < ActionController::TestCase
                                                               :combine_jobs => :script_only,
                                                               :pdata_only => true})
 
-    # bar_file -> hash2
-    assert /fa7aeb5140e2848d39b416daeef4ffc5\+45&#45;&gt;hash2_f866587e2de5291fbd38d616d6d33eab/.match(used_by_svg)
+    stage2 = find_fixture Job, "graph_stage2"
+    stage3 = find_fixture Job, "graph_stage3"
+
+    stage2_id = "#{stage2.script}_#{Digest::MD5.hexdigest(stage2[:script_parameters].to_json)}"
+    stage3_id = "#{stage3.script}_#{Digest::MD5.hexdigest(stage3[:script_parameters].to_json)}"
+
+    obj_id = obj.portable_data_hash.gsub('+', '\\\+')
+    stage3_out = stage3.output.gsub('+', '\\\+')
 
-    # hash -> baz file
-    assert /hash_f866587e2de5291fbd38d616d6d33eab&#45;&gt;ea10d51bcf88862dbcc36eb292017dfd\+45/.match(used_by_svg)
+    assert /#{obj_id}&#45;&gt;#{stage2_id}/.match(used_by_svg)
 
-    # hash2 -> baz file
-    assert /hash2_02a085407e751d00b5dc88f1bd5e8247&#45;&gt;ea10d51bcf88862dbcc36eb292017dfd\+45/.match(used_by_svg)
+    assert /#{obj_id}&#45;&gt;#{stage3_id}/.match(used_by_svg)
 
+    assert /#{stage3_id}&#45;&gt;#{stage3_out}/.match(used_by_svg)
 
-    # File::open "./tmp/stuff4.svg", "w" do |f|
-    #   f.write "<?xml version=\"1.0\" ?>\n"
-    #   f.write used_by_svg
-    # end
+    assert /#{stage3_id}&#45;&gt;#{stage3_out}/.match(used_by_svg)
 
   end
 end

diff --git a/apps/workbench/test/controllers/pipeline_instances_controller_test.rb b/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
index 70b7493db25deec7abceea0ee28da69069efc99c..718b5c4b0561569afb2e4dc0c9bf0741032ed8dd 100644 (file)
--- a/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
+++ b/apps/workbench/test/controllers/pipeline_instances_controller_test.rb
@@ -84,14 +84,19 @@ class PipelineInstancesControllerTest < ActionController::TestCase
     @controller.params['tab_pane'] = "Graph"
     provenance, pips = @controller.graph([pipeline_for_graph])
 
+    graph_test_collection1 = find_fixture Collection, "graph_test_collection1"
+    stage1 = find_fixture Job, "graph_stage1"
+    stage2 = find_fixture Job, "graph_stage2"
+
     ['component_zzzzz-d1hrv-9fm8l10i9z2kqc9_stage1',
      'component_zzzzz-d1hrv-9fm8l10i9z2kqc9_stage2',
-     'zzzzz-8i9sb-graphstage10000',
-     'zzzzz-8i9sb-graphstage20000',
-     'b519d9cb706a29fc7ea24dbea2f05851+93',
-     'fa7aeb5140e2848d39b416daeef4ffc5+45',
-     'zzzzz-4zz18-bv31uwvy3neko22',
-     'zzzzz-4zz18-uukreo9rbgwsujx'].each do |k|
+     stage1.uuid,
+     stage2.uuid,
+     stage1.output,
+     stage2.output,
+     pipeline_for_graph[:components][:stage1][:output_uuid],
+     pipeline_for_graph[:components][:stage2][:output_uuid]
+    ].each do |k|
 
       assert_not_nil provenance[k], "Expected key #{k} in provenance set"
       assert_equal 1, pips[k], "Expected key #{k} in pips set" if !k.start_with? "component_"
@@ -104,16 +109,14 @@ class PipelineInstancesControllerTest < ActionController::TestCase
         :pips => pips,
         :only_components => true }
 
-    # hash -> owned_by_active
-    assert /hash_4fe459abe02d9b365932b8f5dc419439ab4e2577_99914b932bd37a50b983c5e7c90ae93b&#45;&gt;fa7aeb5140e2848d39b416daeef4ffc5\+45/.match(prov_svg)
+    stage1_id = "#{stage1[:script]}_#{stage1[:script_version]}_#{Digest::MD5.hexdigest(stage1[:script_parameters].to_json)}"
+    stage2_id = "#{stage2[:script]}_#{stage2[:script_version]}_#{Digest::MD5.hexdigest(stage2[:script_parameters].to_json)}"
+
+    stage1_out = stage1[:output].gsub('+','\\\+')
 
-    # owned_by_active -> hash2
-    assert /fa7aeb5140e2848d39b416daeef4ffc5\+45&#45;&gt;hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_4900033ec5cfaf8a63566f3664aeaa70/.match(prov_svg)
+    assert_match /#{stage1_id}&#45;&gt;#{stage1_out}/, prov_svg
 
-    #File::open "./tmp/stuff1.svg", "w" do |f|
-    #  f.write "<?xml version=\"1.0\" ?>\n"
-    #  f.write prov_svg
-    #end
+    assert_match /#{stage1_out}&#45;&gt;#{stage2_id}/, prov_svg
 
   end
 
@@ -171,19 +174,25 @@ class PipelineInstancesControllerTest < ActionController::TestCase
     @controller.params['tab_pane'] = "Graph"
     provenance, pips = @controller.graph([pipeline_for_graph1, pipeline_for_graph2])
 
+    collection1 = find_fixture Collection, "graph_test_collection1"
+
+    stage1 = find_fixture Job, "graph_stage1"
+    stage2 = find_fixture Job, "graph_stage2"
+    stage3 = find_fixture Job, "graph_stage3"
+
     [['component_zzzzz-d1hrv-9fm8l10i9z2kqc9_stage1', nil],
      ['component_zzzzz-d1hrv-9fm8l10i9z2kqc9_stage2', nil],
      ['component_zzzzz-d1hrv-9fm8l10i9z2kqc0_stage1', nil],
      ['component_zzzzz-d1hrv-9fm8l10i9z2kqc0_stage2', nil],
-     ['zzzzz-8i9sb-graphstage10000', 3],
-     ['zzzzz-8i9sb-graphstage20000', 1],
-     ['zzzzz-8i9sb-graphstage30000', 2],
-     ['b519d9cb706a29fc7ea24dbea2f05851+93', 1],
-     ['fa7aeb5140e2848d39b416daeef4ffc5+45', 3],
-     ['ea10d51bcf88862dbcc36eb292017dfd+45', 2],
-     ['zzzzz-4zz18-bv31uwvy3neko22', 3],
-     ['zzzzz-4zz18-uukreo9rbgwsujx', 1],
-     ['zzzzz-4zz18-uukreo9rbgwsujj', 2]
+     [stage1.uuid, 3],
+     [stage2.uuid, 1],
+     [stage3.uuid, 2],
+     [stage1.output, 3],
+     [stage2.output, 1],
+     [stage3.output, 2],
+     [pipeline_for_graph1[:components][:stage1][:output_uuid], 3],
+     [pipeline_for_graph1[:components][:stage2][:output_uuid], 1],
+     [pipeline_for_graph2[:components][:stage2][:output_uuid], 2]
     ].each do |k|
       assert_not_nil provenance[k[0]], "Expected key #{k[0]} in provenance set"
       assert_equal k[1], pips[k[0]], "Expected key #{k} in pips" if !k[0].start_with? "component_"
@@ -196,22 +205,19 @@ class PipelineInstancesControllerTest < ActionController::TestCase
         :pips => pips,
         :only_components => true }
 
-    # owned_by_active -> hash2 (stuff)
-    assert /fa7aeb5140e2848d39b416daeef4ffc5\+45&#45;&gt;hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_4900033ec5cfaf8a63566f3664aeaa70/.match(prov_svg)
+    collection1_id = collection1.portable_data_hash.gsub('+','\\\+')
 
-    # owned_by_active -> hash2 (stuff2)
-    assert /fa7aeb5140e2848d39b416daeef4ffc5\+45&#45;&gt;hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_02a085407e751d00b5dc88f1bd5e8247/.match(prov_svg)
+    stage2_id = "#{stage2[:script]}_#{stage2[:script_version]}_#{Digest::MD5.hexdigest(stage2[:script_parameters].to_json)}"
+    stage3_id = "#{stage3[:script]}_#{stage3[:script_version]}_#{Digest::MD5.hexdigest(stage3[:script_parameters].to_json)}"
 
-    # hash2 (stuff) -> GPL
-    assert /hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_4900033ec5cfaf8a63566f3664aeaa70&#45;&gt;b519d9cb706a29fc7ea24dbea2f05851\+93/.match(prov_svg)
+    stage2_out = stage2[:output].gsub('+','\\\+')
+    stage3_out = stage3[:output].gsub('+','\\\+')
 
-    # hash2 (stuff2) -> baz file
-    assert /hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_02a085407e751d00b5dc88f1bd5e8247&#45;&gt;ea10d51bcf88862dbcc36eb292017dfd\+45/.match(prov_svg)
+    assert_match /#{collection1_id}&#45;&gt;#{stage2_id}/, prov_svg
+    assert_match /#{collection1_id}&#45;&gt;#{stage3_id}/, prov_svg
 
-    # File::open "./tmp/stuff2.svg", "w" do |f|
-    #   f.write "<?xml version=\"1.0\" ?>\n"
-    #   f.write prov_svg
-    # end
+    assert_match /#{stage2_id}&#45;&gt;#{stage2_out}/, prov_svg
+    assert_match /#{stage3_id}&#45;&gt;#{stage3_out}/, prov_svg
 
   end
 

diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb
index 3d7c34812f21910f8501b129f8c19bab443ac6db..6f95b3d0d0173ae01eb2a29c90f37d9de1c5a40e 100644 (file)
--- a/apps/workbench/test/integration/pipeline_instances_test.rb
+++ b/apps/workbench/test/integration/pipeline_instances_test.rb
@@ -134,7 +134,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest
     # since the pipeline component has a job, expect to see the graph
     assert page.has_text? 'Graph'
     click_link 'Graph'
-    assert page.has_text? 'script_version'
+    page.assert_selector "#provenance_graph"
   end
 
   test 'pipeline description' do

diff --git a/services/api/app/controllers/arvados/v1/collections_controller.rb b/services/api/app/controllers/arvados/v1/collections_controller.rb
index dbcc0462dc8208e064cbbf51068e22dd2ae0ce3b..f546a4afe2a1e736261e90501e91fc5dd71360bb 100644 (file)
--- a/services/api/app/controllers/arvados/v1/collections_controller.rb
+++ b/services/api/app/controllers/arvados/v1/collections_controller.rb
@@ -76,15 +76,16 @@ class Arvados::V1::CollectionsController < ApplicationController
 
     if loc
       # uuid is a portable_data_hash
-      c = Collection.readable_by(*@read_users).where(portable_data_hash: loc.to_s).all
+      collections = Collection.readable_by(*@read_users).where(portable_data_hash: loc.to_s)
+      c = collections.limit(2).all
       if c.size == 1
         visited[loc.to_s] = c[0]
       elsif c.size > 1
-        named = c.select {|n| not n.name.nil? and not n.name.empty? }
-        if named.any?
+        name = collections.limit(1).where("name <> ''").first
+        if name
           visited[loc.to_s] = {
             portable_data_hash: c[0].portable_data_hash,
-            name: "#{named[0].name} + #{c.size-1} more"
+            name: "#{name.name} + #{collections.count-1} more"
           }
         else
           visited[loc.to_s] = {