[ARVADOS] updated: 488a811374ff4bdeed9f2f2f57d9ef31d9369b5b

git at public.curoverse.com git at public.curoverse.com
Fri Jun 6 15:55:35 EDT 2014


Summary of changes:
 .../app/controllers/application_controller.rb      | 13 +++++-
 sdk/cli/bin/arv-run-pipeline-instance              |  2 +-
 .../app/controllers/arvados/v1/nodes_controller.rb | 12 ++++--
 services/api/app/models/blob.rb                    | 13 +-----
 services/api/app/models/node.rb                    |  9 ++++
 services/api/script/cancel_stale_jobs.rb           | 37 +++++++++++++++++
 services/api/script/crunch-dispatch.rb             | 34 ++++-----------
 services/api/test/fixtures/nodes.yml               |  1 +
 .../arvados/v1/collections_controller_test.rb      | 48 ++++++++++------------
 .../functional/arvados/v1/nodes_controller_test.rb | 16 ++++++++
 services/api/test/unit/node_test.rb                | 22 ++++++++--
 11 files changed, 136 insertions(+), 71 deletions(-)
 create mode 100755 services/api/script/cancel_stale_jobs.rb

       via  488a811374ff4bdeed9f2f2f57d9ef31d9369b5b (commit)
       via  2f3d49bde80526060d3337f13dfa91cd581ac222 (commit)
       via  6a8463eea9e32cb5cffcaf2f04667520794404ec (commit)
       via  fa87d357fdefd594b378cda9b4d73487df60d262 (commit)
       via  1f760632ae0fbb9f11af5cfb831b7c3ed49a7009 (commit)
       via  428973c03d4b4cd96adc80a514beffbb739d987a (commit)
       via  35c20b4ad8220131f7f6bad6b3806a7d28df3ef3 (commit)
       via  f48482bd37d3ae5a5f1aa488fa330f77c5fd640d (commit)
       via  0e7e57bbd8030c8144a18e43e68945ab11ad094c (commit)
       via  0b4f867ef14af38c07b910643fbe8cc6a93e6bb6 (commit)
       via  a163b0c7b5afa782281f67f0fa0ca0e4b41c518f (commit)
       via  a46f0152c44fe20eba4db38858eaa2f99bae83f2 (commit)
       via  1ec252c8087c1f167d969e26c584ff346f4ac457 (commit)
       via  ae09643622ecea00bab110f20029f01c83e1cf30 (commit)
       via  2a64eae3cf8363c596feda5337ea20ce356ca11f (commit)
       via  114df81b90be76e6921b9f20c9ddb272567c82e1 (commit)
       via  a276e40691a8f96b321879de2279159ef08b804f (commit)
       via  cb77d123755112d17f2e7bb2bd869d957d8a00f3 (commit)
      from  cea92754dfacf2b409d1f5b45dd0775fc44c842d (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 488a811374ff4bdeed9f2f2f57d9ef31d9369b5b
Author: Tim Pierce <twp at curoverse.com>
Date:   Fri Jun 6 15:48:43 2014 -0400

    2857: strip signatures from manifests before testing equality.
    
    As Brett observed, the "post" method in these tests mutates its
    argument. For a valid test, we must ensure that the source data has not
    been transformed into the result data.
    
    For tests which do not care about validating the signatures, we just
    strip out the signatures from the result manifest before comparing it to
    the source.
    
    Refs #2857.

diff --git a/services/api/app/models/blob.rb b/services/api/app/models/blob.rb
index c4e1881..5decd77 100644
--- a/services/api/app/models/blob.rb
+++ b/services/api/app/models/blob.rb
@@ -16,15 +16,6 @@ class Blob
   class InvalidSignatureError < StandardError
   end
 
-  # Clock is a wrapper for Time.
-  # It can be replaced with a stub for unit testing (the poor man's mock).
-  # It must support a Clock.now method that returns a Time object.
-  @@Clock = Time
-
-  def self.set_clock c
-    @@Clock = c
-  end
-
   # Blob.sign_locator: return a signed and timestamped blob locator.
   #
   # The 'opts' argument should include:
@@ -44,7 +35,7 @@ class Blob
       end
       timestamp = opts[:expire]
     else
-      timestamp = @@Clock.now.to_i + (opts[:ttl] || 600)
+      timestamp = Time.now.to_i + (opts[:ttl] || 600)
     end
     timestamp_hex = timestamp.to_s(16)
     # => "53163cb4"
@@ -91,7 +82,7 @@ class Blob
     if !timestamp.match /^[\da-f]+$/
       raise Blob::InvalidSignatureError.new 'Timestamp is not a base16 number.'
     end
-    if timestamp.to_i(16) < @@Clock.now.to_i
+    if timestamp.to_i(16) < Time.now.to_i
       raise Blob::InvalidSignatureError.new 'Signature expiry time has passed.'
     end
 
diff --git a/services/api/test/functional/arvados/v1/collections_controller_test.rb b/services/api/test/functional/arvados/v1/collections_controller_test.rb
index ba6929b..6830c6b 100644
--- a/services/api/test/functional/arvados/v1/collections_controller_test.rb
+++ b/services/api/test/functional/arvados/v1/collections_controller_test.rb
@@ -2,25 +2,6 @@ require 'test_helper'
 
 class Arvados::V1::CollectionsControllerTest < ActionController::TestCase
 
-  # StoppedClock.now always returns the same timestamp.
-  # Set the Blob permission signing clock to ensure that
-  # all permission hints use consistent timestamps for testing.
-
-  class StoppedClock
-    @@cached_timestamp = Time.now
-    def self.now
-      return @@cached_timestamp
-    end
-  end
-
-  def setup
-    Blob.set_clock(StoppedClock)
-  end
-
-  def teardown
-    Blob.set_clock(Time)
-  end
-
   test "should get index" do
     authorize_with :active
     get :index
@@ -75,9 +56,14 @@ EOS
       Digest::MD5.hexdigest(test_collection[:manifest_text]) +
       '+' +
       test_collection[:manifest_text].length.to_s
+
+    # post :create will modify test_collection in place, so we save a copy first.
+    # Hash.deep_dup is not sufficient as it preserves references of strings (??!?)
+    post_collection = Marshal.load(Marshal.dump(test_collection))
     post :create, {
-      collection: test_collection
+      collection: post_collection
     }
+
     assert_response :success
     assert_nil assigns(:objects)
 
@@ -88,7 +74,11 @@ EOS
     assert_not_nil assigns(:object)
     resp = JSON.parse(@response.body)
     assert_equal test_collection[:uuid], resp['uuid']
-    assert_equal test_collection[:manifest_text], resp['manifest_text']
+
+    # The manifest in the response will have had permission hints added.
+    # Remove any permission hints in the response before comparing it to the source.
+    stripped_manifest = resp['manifest_text'].gsub(/\+A[A-Za-z0-9 at _-]+/, '')
+    assert_equal test_collection[:manifest_text], stripped_manifest
     assert_equal 9, resp['data_size']
     assert_equal [['.', 'foo.txt', 0],
                   ['.', 'bar.txt', 6],
@@ -412,18 +402,24 @@ EOS
       '+' +
       manifest_text.length.to_s
 
+    test_collection = {
+      manifest_text: manifest_text,
+      uuid: manifest_uuid,
+    }
+    post_collection = Marshal.load(Marshal.dump(test_collection))
     post :create, {
-      collection: {
-        manifest_text: manifest_text,
-        uuid: manifest_uuid,
-      }
+      collection: post_collection
     }
     assert_response :success
     assert_not_nil assigns(:object)
     resp = JSON.parse(@response.body)
     assert_equal manifest_uuid, resp['uuid']
     assert_equal 48, resp['data_size']
-    assert_equal resp['manifest_text'], manifest_text
+
+    # The manifest in the response will have had permission hints added.
+    # Remove any permission hints in the response before comparing it to the source.
+    stripped_manifest = resp['manifest_text'].gsub(/\+A[A-Za-z0-9 at _-]+/, '')
+    assert_equal manifest_text, stripped_manifest
   end
 
   test "multiple signed locators per line" do

commit 2f3d49bde80526060d3337f13dfa91cd581ac222
Merge: cea9275 6a8463e
Author: Tim Pierce <twp at curoverse.com>
Date:   Fri Jun 6 11:31:59 2014 -0400

    Merge branch 'master' into 2857-collection-tests


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


hooks/post-receive
-- 




More information about the arvados-commits mailing list