From 15dbb3110b3b7cc6712e58bdfbb9a5df7fab1882 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 3 Nov 2014 15:06:09 -0500 Subject: [PATCH] 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. --- .../app/controllers/jobs_controller.rb | 4 - .../app/helpers/provenance_helper.rb | 8 +- .../collections_controller_test.rb | 49 ++++++------ .../pipeline_instances_controller_test.rb | 76 ++++++++++--------- .../integration/pipeline_instances_test.rb | 2 +- .../arvados/v1/collections_controller.rb | 9 ++- 6 files changed, 77 insertions(+), 71 deletions(-) diff --git a/apps/workbench/app/controllers/jobs_controller.rb b/apps/workbench/app/controllers/jobs_controller.rb index 00ce0a5382..536518277f 100644 --- 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 89d9ee6ceb..e8850d5b47 100644 --- 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(/ :bottom_up, :combine_jobs => :script_only}) - # hash -> baz file - assert /ea10d51bcf88862dbcc36eb292017dfd\+45->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->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->fa7aeb5140e2848d39b416daeef4ffc5\+45/.match(prov_svg) + assert /#{obj_id}->#{stage3_id}/.match(prov_svg) - # owned_by_active -> hash2 - assert /hash2_02a085407e751d00b5dc88f1bd5e8247->fa7aeb5140e2848d39b416daeef4ffc5\+45/.match(prov_svg) + assert /#{stage3_id}->#{stage1_out}/.match(prov_svg) - # File::open "./tmp/stuff3.svg", "w" do |f| - # f.write "\n" - # f.write prov_svg - # end + assert /#{stage1_out}->#{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->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->ea10d51bcf88862dbcc36eb292017dfd\+45/.match(used_by_svg) + assert /#{obj_id}->#{stage2_id}/.match(used_by_svg) - # hash2 -> baz file - assert /hash2_02a085407e751d00b5dc88f1bd5e8247->ea10d51bcf88862dbcc36eb292017dfd\+45/.match(used_by_svg) + assert /#{obj_id}->#{stage3_id}/.match(used_by_svg) + assert /#{stage3_id}->#{stage3_out}/.match(used_by_svg) - # File::open "./tmp/stuff4.svg", "w" do |f| - # f.write "\n" - # f.write used_by_svg - # end + assert /#{stage3_id}->#{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 70b7493db2..718b5c4b05 100644 --- 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->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->hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_4900033ec5cfaf8a63566f3664aeaa70/.match(prov_svg) + assert_match /#{stage1_id}->#{stage1_out}/, prov_svg - #File::open "./tmp/stuff1.svg", "w" do |f| - # f.write "\n" - # f.write prov_svg - #end + assert_match /#{stage1_out}->#{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->hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_4900033ec5cfaf8a63566f3664aeaa70/.match(prov_svg) + collection1_id = collection1.portable_data_hash.gsub('+','\\\+') - # owned_by_active -> hash2 (stuff2) - assert /fa7aeb5140e2848d39b416daeef4ffc5\+45->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->b519d9cb706a29fc7ea24dbea2f05851\+93/.match(prov_svg) + stage2_out = stage2[:output].gsub('+','\\\+') + stage3_out = stage3[:output].gsub('+','\\\+') - # hash2 (stuff2) -> baz file - assert /hash2_4fe459abe02d9b365932b8f5dc419439ab4e2577_02a085407e751d00b5dc88f1bd5e8247->ea10d51bcf88862dbcc36eb292017dfd\+45/.match(prov_svg) + assert_match /#{collection1_id}->#{stage2_id}/, prov_svg + assert_match /#{collection1_id}->#{stage3_id}/, prov_svg - # File::open "./tmp/stuff2.svg", "w" do |f| - # f.write "\n" - # f.write prov_svg - # end + assert_match /#{stage2_id}->#{stage2_out}/, prov_svg + assert_match /#{stage3_id}->#{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 3d7c34812f..6f95b3d0d0 100644 --- 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 dbcc0462dc..f546a4afe2 100644 --- 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] = { -- 2.30.2