6859: Fix up module usage, mock backtick to test arv-put error handling.

diff --git a/services/api/lib/salvage_collection.rb b/services/api/lib/salvage_collection.rb
index 86c95c67729de09940b70a4f94371c11ca2c1981..79a2d58bc628792ccc24cec0ede78bf76d3dc142 100755 (executable)
--- a/services/api/lib/salvage_collection.rb
+++ b/services/api/lib/salvage_collection.rb
@@ -14,7 +14,7 @@ module SalvageCollection
   require 'tempfile'
   require 'shellwords'
 
-  def self.salvage_collection_arv_put cmd
+  def salvage_collection_arv_put cmd
     new_manifest = %x(#{cmd})
     if $?.success?
       new_manifest
@@ -25,7 +25,7 @@ module SalvageCollection
 
   # Get all the locators from the original manifest
   LOCATOR_REGEXP = /((.*))?([[:xdigit:]]{32})(\+(.*))?\z/
-  def self.salvage_collection_locator_data manifest
+  def salvage_collection_locator_data manifest
       locators = []
       size = 0
       manifest.each_line do |line|
@@ -50,7 +50,7 @@ module SalvageCollection
       return [locators, size]
   end
 
-  def self.salvage_collection uuid, reason='salvaged - see #6277, #6859'
+  def salvage_collection uuid, reason='salvaged - see #6277, #6859'
     act_as_system_user do
       if !ENV['ARVADOS_API_TOKEN'].present? or !ENV['ARVADOS_API_HOST'].present?
         raise "ARVADOS environment variables missing. Please set your admin user credentials as ARVADOS environment variables."

diff --git a/services/api/test/unit/salvage_collection_test.rb b/services/api/test/unit/salvage_collection_test.rb
index 96b2c3e747ae1a6169fc04ae1ac2676b1476ee85..47abe5a0d26b686196a1bb9442600319a262b25c 100644 (file)
--- a/services/api/test/unit/salvage_collection_test.rb
+++ b/services/api/test/unit/salvage_collection_test.rb
@@ -1,5 +1,6 @@
 require 'test_helper'
 require 'salvage_collection'
+require 'shellwords'
 
 # Valid manifest_text
 TEST_MANIFEST = ". 341dabea2bd78ad0d6fc3f5b926b450e+85626+Ad391622a17f61e4a254eda85d1ca751c4f368da9@55e076ce 0:85626:brca2-hg19.fa\n. d7321a918923627c972d8f8080c07d29+82570+A22e0a1d9b9bc85c848379d98bedc64238b0b1532@55e076ce 0:82570:brca1-hg19.fa\n"
@@ -16,21 +17,6 @@ TEST_MANIFEST_STRIPPED = ". 341dabea2bd78ad0d6fc3f5b926b450e+85626 0:85626:brca2
 # Expectation: All these locators are preserved in salvaged_data
 BAD_MANIFEST = "faaaafaafaafaabd78ad0d6fc3f5b926b450e+foo bar-baabaabaabd78ad0d6fc3f5b926b450e bad12345dae58ad0d6fc3f5b926b450e+ 341dabea2bd78ad0d6fc3f5b926b450e+abc 341dabea2bd78ad0d6fc3f5b926abcdf 0:85626:brca2-hg19.fa\n. abcdabea2bd78ad0d6fc3f5b926b450e+1000 0:1000:brca-hg19.fa\n. d7321a918923627c972d8f8080c07d29+2000+A22e0a1d9b9bc85c848379d98bedc64238b0b1532@55e076ce 0:2000:brca1-hg19.fa\n"
 
-# Mock arv_put
-module SalvageCollection
-  def self.salvage_collection_arv_put(cmd)
-    file_contents = File.open(cmd.split[-1], "r").read
-    # simulate arv-put error when it is 'user_agreement'
-    if file_contents.include? 'GNU_General_Public_License'
-      raise("Error during arv-put")
-    else
-      ". " +
-      Digest::MD5.hexdigest(file_contents) + "+" + file_contents.length.to_s +
-      " 0:" + file_contents.length.to_s + ":invalid_manifest_text.txt\n"
-    end
-  end
-end
-
 class SalvageCollectionTest < ActiveSupport::TestCase
   include SalvageCollection
 
@@ -39,6 +25,7 @@ class SalvageCollectionTest < ActiveSupport::TestCase
     # arv-put needs ARV env variables
     ENV['ARVADOS_API_HOST'] = 'unused_by_test'
     ENV['ARVADOS_API_TOKEN'] = 'unused_by_test'
+    @backtick_mock_failure = false
   end
 
   teardown do
@@ -46,13 +33,27 @@ class SalvageCollectionTest < ActiveSupport::TestCase
     ENV['ARVADOS_API_TOKEN'] = ''
   end
 
+  def ` cmd # mock Kernel `
+    assert_equal 'arv-put', cmd.shellsplit[0]
+    if @backtick_mock_failure
+      # run a process so $? indicates failure
+      return super 'false'
+    end
+    # run a process so $? indicates success
+    super 'true'
+    file_contents = File.open(cmd.shellsplit[-1], "r").read
+    ". " +
+      Digest::MD5.hexdigest(file_contents) + "+" + file_contents.length.to_s +
+      " 0:" + file_contents.length.to_s + ":invalid_manifest_text.txt\n"
+  end
+
   test "salvage test collection with valid manifest text" do
     # create a collection to test salvaging
     src_collection = Collection.new name: "test collection", manifest_text: TEST_MANIFEST
     src_collection.save!
 
     # salvage this collection
-    SalvageCollection.salvage_collection src_collection.uuid, 'test salvage collection - see #6277, #6859'
+    salvage_collection src_collection.uuid, 'test salvage collection - see #6277, #6859'
 
     # verify the updated src_collection data
     updated_src_collection = Collection.find_by_uuid src_collection.uuid
@@ -80,13 +81,13 @@ class SalvageCollectionTest < ActiveSupport::TestCase
 
   test "salvage collection with no uuid required argument" do
     e = assert_raises RuntimeError do
-      SalvageCollection.salvage_collection nil
+      salvage_collection nil
     end
   end
 
   test "salvage collection with bogus uuid" do
     e = assert_raises RuntimeError do
-      SalvageCollection.salvage_collection 'bogus-uuid'
+      salvage_collection 'bogus-uuid'
     end
     assert_equal "No collection found for bogus-uuid.", e.message
   end
@@ -95,17 +96,18 @@ class SalvageCollectionTest < ActiveSupport::TestCase
     e = assert_raises RuntimeError do
       ENV['ARVADOS_API_HOST'] = ''
       ENV['ARVADOS_API_TOKEN'] = ''
-      SalvageCollection.salvage_collection collections('user_agreement').uuid
+      salvage_collection collections('user_agreement').uuid
     end
     assert_equal "ARVADOS environment variables missing. Please set your admin user credentials as ARVADOS environment variables.", e.message
   end
 
   test "salvage collection with error during arv-put" do
     # try to salvage collection while mimicking error during arv-put
+    @backtick_mock_failure = true
     e = assert_raises RuntimeError do
-      SalvageCollection.salvage_collection collections('user_agreement').uuid
+      salvage_collection collections('user_agreement').uuid
     end
-    assert_equal "Error during arv-put", e.message
+    assert_match /Error during arv-put: pid \d+ exit \d+ \(cmd was \"arv-put .*\"\)/, e.message
   end
 
   # This test uses BAD_MANIFEST, which has the following flaws:
@@ -118,7 +120,7 @@ class SalvageCollectionTest < ActiveSupport::TestCase
   #     341dabea2bd78ad0d6fc3f5b926abcdf
   # Expectation: All these locators are preserved in salvaged_data
   test "invalid locators preserved during salvaging" do
-    locator_data = SalvageCollection.salvage_collection_locator_data BAD_MANIFEST
+    locator_data = salvage_collection_locator_data BAD_MANIFEST
     assert_equal 7, locator_data[0].size
     assert_equal false, locator_data[0].include?("foo-faafaafaabd78ad0d6fc3f5b926b450e+foo")
     assert_equal true,  locator_data[0].include?("faafaafaabd78ad0d6fc3f5b926b450e")
@@ -140,7 +142,7 @@ class SalvageCollectionTest < ActiveSupport::TestCase
     src_collection.save!(validate: false)
 
     # salvage this collection
-    SalvageCollection.salvage_collection src_collection.uuid, 'test salvage collection - see #6277, #6859'
+    salvage_collection src_collection.uuid, 'test salvage collection - see #6277, #6859'
 
     # verify the updated src_collection data
     updated_src_collection = Collection.find_by_uuid src_collection.uuid