[ARVADOS] updated: dad9a13b4942f50e9a05d796281770de75eb8660
git at public.curoverse.com
git at public.curoverse.com
Tue Mar 10 13:24:04 EDT 2015
Summary of changes:
sdk/ruby/lib/arvados/collection.rb | 280 +++++++++++++++++++++++++------------
sdk/ruby/test/test_collection.rb | 265 ++++++++++++++++++++++++-----------
2 files changed, 377 insertions(+), 168 deletions(-)
via dad9a13b4942f50e9a05d796281770de75eb8660 (commit)
from 64046fa6553461bed50d36f585ea1762e4ff6fdd (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 dad9a13b4942f50e9a05d796281770de75eb8660
Author: Brett Smith <brett at curoverse.com>
Date: Mon Mar 9 15:03:42 2015 -0400
5104: Implement code review feedback.
diff --git a/sdk/ruby/lib/arvados/collection.rb b/sdk/ruby/lib/arvados/collection.rb
index 94f5138..67c0580 100644
--- a/sdk/ruby/lib/arvados/collection.rb
+++ b/sdk/ruby/lib/arvados/collection.rb
@@ -5,56 +5,63 @@ module Arv
def initialize(manifest_text="")
@tree = CollectionStream.new(".")
@manifest_text = ""
- import_manifest!(manifest_text)
+ @modified = false
+ import_manifest(manifest_text)
end
def manifest_text
@manifest_text ||= @tree.manifest_text
end
- def import_manifest!(manifest_text)
+ def import_manifest(manifest_text)
manifest = Keep::Manifest.new(manifest_text)
manifest.each_line do |stream_root, locators, file_specs|
if stream_root.empty? or locators.empty? or file_specs.empty?
raise ArgumentError.new("manifest text includes malformed line")
end
+ loc_list = LocatorList.new(locators)
file_specs.map { |s| manifest.split_file_token(s) }.
each do |file_start, file_len, file_path|
@tree.file_at(normalize_path(stream_root, file_path)).
- add_range(locators, file_start, file_len)
+ add_segment(loc_list.segment(file_start, file_len))
end
end
if @manifest_text == ""
@manifest_text = manifest_text
- self
else
- modified!
+ modified
end
+ self
+ end
+
+ def modified?
+ @modified
end
- def normalize!
- # We generate normalized manifests, so all we have to do is force
- # regeneration.
- modified!
+ def unmodified
+ @modified = false
+ self
end
- def copy!(source, target, source_collection=nil)
+ def normalize
+ @manifest_text = @tree.manifest_text
+ self
+ end
+
+ def cp_r(source, target, source_collection=nil)
copy(:merge, source, target, source_collection)
end
- def rename!(source, target)
- copy(:add_copy, source, target) { remove!(source, recursive: true) }
+ def rename(source, target)
+ copy(:add_copy, source, target) { rm_r(source) }
end
- def remove!(path, opts={})
- stream, name = find(path)
- if name.nil?
- return self if @tree.leaf?
- @tree = CollectionStream.new(".")
- else
- stream.delete(name, opts)
- end
- modified!
+ def rm(source)
+ remove(source)
+ end
+
+ def rm_r(source)
+ remove(source, recursive: true)
end
protected
@@ -70,6 +77,18 @@ module Arv
private
+ def modified
+ @manifest_text = nil
+ @modified = true
+ self
+ end
+
+ def normalize_path(*parts)
+ path = File.join(*parts)
+ raise ArgumentError.new("empty path") if path.empty?
+ path.sub(/^\.(\/|$)/, "")
+ end
+
def copy(copy_method, source, target, source_collection=nil)
# Find the item at path `source` in `source_collection`, find the
# destination stream at path `target`, and use `copy_method` to copy
@@ -119,18 +138,136 @@ module Arv
end
end
dst_stream.send(copy_method, src_item, target_name)
- modified!
+ modified
end
- def modified!
- @manifest_text = nil
- self
+ def remove(path, opts={})
+ stream, name = find(path)
+ if name.nil?
+ if not opts[:recursive]
+ raise Errno::EISDIR.new(@tree.path)
+ elsif @tree.leaf?
+ return self
+ else
+ @tree = CollectionStream.new(".")
+ end
+ else
+ stream.delete(name, opts)
+ end
+ modified
end
- def normalize_path(*parts)
- path = File.join(*parts)
- raise ArgumentError.new("empty path") if path.empty?
- path.sub(/^\.(\/|$)/, "")
+ LocatorSegment = Struct.new(:locators, :start_pos, :length)
+
+ class LocatorRange < Range
+ attr_reader :locator
+
+ def initialize(loc_s, start)
+ @locator = loc_s
+ range_end = start + Keep::Locator.parse(loc_s).size.to_i
+ super(start, range_end, false)
+ end
+ end
+
+ class LocatorList
+ def initialize(locators=[])
+ @ranges = []
+ @loc_ranges = {}
+ @last_loc_range = nil
+ extend(locators)
+ end
+
+ def manifest_s
+ @loc_ranges.keys.join(" ")
+ end
+
+ def extend(locators)
+ locators.each do |loc_s|
+ @ranges << new_range_after(@ranges.last, loc_s)
+ unless @loc_ranges.include?(loc_s)
+ @loc_ranges[loc_s] = new_range_after(@last_loc_range, loc_s)
+ @last_loc_range = @loc_ranges[loc_s]
+ end
+ end
+ end
+
+ def segment(start_pos, length)
+ # Return a LocatorSegment that captures `length` bytes from `start_pos`.
+ start_index = search_for_byte(start_pos)
+ if length == 0
+ end_index = start_index
+ else
+ end_index = search_for_byte(start_pos + length - 1, start_index)
+ end
+ seg_ranges = @ranges[start_index..end_index]
+ LocatorSegment.new(seg_ranges.map(&:locator),
+ start_pos - seg_ranges.first.begin,
+ length)
+ end
+
+ def specs_for(filename, segment)
+ # Given an escaped filename and a LocatorSegment, add the
+ # locators stored in the Segment, then return the smallest
+ # possible array of file spec strings to build the file from
+ # locators in the list.
+ extend(segment.locators)
+ start_pos = segment.start_pos
+ length = segment.length
+ start_loc = segment.locators.first
+ prev_loc = start_loc
+ result = []
+ # Build a list of file specs by iterating through the segment's
+ # locators and preparing a file spec for each contiguous range.
+ segment.locators[1..-1].each do |loc_s|
+ range = @loc_ranges[loc_s]
+ if range.begin != @loc_ranges[prev_loc].end
+ range_start, range_length =
+ start_and_length_at(start_loc, prev_loc, start_pos, length)
+ result << "#{range_start}:#{range_length}:#{filename}"
+ start_pos = 0
+ length -= range_length
+ start_loc = loc_s
+ end
+ prev_loc = loc_s
+ end
+ range_start, range_length =
+ start_and_length_at(start_loc, prev_loc, start_pos, length)
+ result << "#{range_start}:#{range_length}:#{filename}"
+ result
+ end
+
+ private
+
+ def new_range_after(prev_range, loc_s)
+ LocatorRange.new(loc_s, (prev_range.nil?) ? 0 : prev_range.end)
+ end
+
+ def search_for_byte(target, start_index=0)
+ # Do a binary search for byte `target` in the list of locators,
+ # starting from `start_index`. Return the index of the range in
+ # @ranges that contains the byte.
+ lo = start_index
+ hi = @ranges.size
+ loop do
+ ii = (lo + hi) / 2
+ range = @ranges[ii]
+ if range.include?(target)
+ return ii
+ elsif ii == lo
+ raise RangeError.new("%i not in segment" % target)
+ elsif target < range.begin
+ hi = ii
+ else
+ lo = ii
+ end
+ end
+ end
+
+ def start_and_length_at(start_key, end_key, start_pos, length)
+ range_begin = @loc_ranges[start_key].begin + start_pos
+ range_length = [@loc_ranges[end_key].end - range_begin, length].min
+ [range_begin, range_length]
+ end
end
class CollectionItem
@@ -142,35 +279,30 @@ module Arv
end
end
- LocatorRange = Struct.new(:locators, :start_pos, :length)
-
class CollectionFile < CollectionItem
def initialize(path)
super
- @ranges = []
+ @segments = []
end
def self.human_name
"file"
end
+ def file?
+ true
+ end
+
def leaf?
true
end
- def add_range(locators, start_pos, length)
- # Given an array of locators, and this file's start position and
- # length within them, store a LocatorRange with information about
- # the locators actually used.
- loc_sizes = locators.map { |s| Keep::Locator.parse(s).size.to_i }
- start_index, start_pos = loc_size_index(loc_sizes, start_pos, 0, :>=)
- end_index, _ = loc_size_index(loc_sizes, length, start_index, :>)
- @ranges << LocatorRange.
- new(locators[start_index..end_index], start_pos, length)
+ def add_segment(segment)
+ @segments << segment
end
- def each_range(&block)
- @ranges.each(&block)
+ def each_segment(&block)
+ @segments.each(&block)
end
def check_can_add_copy(src_item, name)
@@ -181,23 +313,9 @@ module Arv
def copy_named(copy_path)
copy = self.class.new(copy_path)
- each_range { |range| copy.add_range(*range) }
+ each_segment { |segment| copy.add_segment(segment) }
copy
end
-
- private
-
- def loc_size_index(loc_sizes, length, index, comp_op)
- # Pass in an array of locator size hints (integers). Starting from
- # `index`, step through the size array until they provide a number
- # of bytes that is `comp_op` (:>= or :>) to `length`. Return the
- # index of the end locator and the amount of data to read from it.
- while length.send(comp_op, loc_sizes[index])
- index += 1
- length -= loc_sizes[index]
- end
- [index, length]
- end
end
class CollectionStream < CollectionItem
@@ -210,6 +328,10 @@ module Arv
"stream"
end
+ def file?
+ false
+ end
+
def leaf?
items.empty?
end
@@ -221,10 +343,10 @@ module Arv
def delete(name, opts={})
item = self[name]
- if item.leaf? or opts[:recursive]
+ if item.file? or opts[:recursive]
items.delete(name)
else
- raise Errno::ENOTEMPTY.new(path)
+ raise Errno::EISDIR.new(path)
end
end
@@ -260,7 +382,7 @@ module Arv
# Return a string with the normalized manifest text for this stream,
# including all substreams.
file_keys, stream_keys = items.keys.sort.partition do |key|
- items[key].is_a?(CollectionFile)
+ items[key].file?
end
my_line = StreamManifest.new(path)
file_keys.each do |file_name|
@@ -344,17 +466,19 @@ module Arv
class StreamManifest
# Build a manifest text for a single stream, without substreams.
+ # The manifest includes files in the order they're added. If you want
+ # a normalized manifest, add files in lexical order by name.
def initialize(name)
@name = name
- @locators = []
- @loc_sizes = []
+ @locators = LocatorList.new
@file_specs = []
end
def add_file(coll_file)
- coll_file.each_range do |range|
- add(coll_file.name, *range)
+ coll_file.each_segment do |segment|
+ @file_specs += @locators.specs_for(escape_name(coll_file.name),
+ segment)
end
end
@@ -362,37 +486,13 @@ module Arv
if @file_specs.empty?
""
else
- "%s %s %s\n" % [escape_name(@name), @locators.join(" "),
+ "%s %s %s\n" % [escape_name(@name), @locators.manifest_s,
@file_specs.join(" ")]
end
end
private
- def add(file_name, loc_a, file_start, file_len)
- # Ensure that the locators in loc_a appear in this locator in sequence,
- # adding as few as possible. Save a new file spec based on those
- # locators' position.
- loc_size = @locators.size
- add_size = loc_a.size
- loc_ii = 0
- add_ii = 0
- while (loc_ii < loc_size) and (add_ii < add_size)
- if @locators[loc_ii] == loc_a[add_ii]
- add_ii += 1
- else
- add_ii = 0
- end
- loc_ii += 1
- end
- loc_ii -= add_ii
- to_add = loc_a[add_ii, add_size] || []
- @locators += to_add
- @loc_sizes += to_add.map { |s| Keep::Locator.parse(s).size.to_i }
- start = @loc_sizes[0, loc_ii].reduce(0, &:+) + file_start
- @file_specs << "#{start}:#{file_len}:#{escape_name(file_name)}"
- end
-
def escape_name(name)
name.gsub(/\\/, "\\\\\\\\").gsub(/\s/) do |s|
s.each_byte.map { |c| "\\%03o" % c }.join("")
diff --git a/sdk/ruby/test/test_collection.rb b/sdk/ruby/test/test_collection.rb
index 5fba278..e31b009 100644
--- a/sdk/ruby/test/test_collection.rb
+++ b/sdk/ruby/test/test_collection.rb
@@ -50,22 +50,22 @@ class CollectionTest < Minitest::Test
def test_no_implicit_normalization_from_first_import
coll = Arv::Collection.new
- coll.import_manifest!(NONNORMALIZED_MANIFEST)
+ coll.import_manifest(NONNORMALIZED_MANIFEST)
assert_equal(NONNORMALIZED_MANIFEST, coll.manifest_text)
end
- ### .import_manifest!
+ ### .import_manifest
def test_non_posix_path_handling
block = random_block(9)
coll = Arv::Collection.new("./.. #{block} 0:5:.\n")
- coll.import_manifest!("./.. #{block} 5:4:..\n")
+ coll.import_manifest("./.. #{block} 5:4:..\n")
assert_equal("./.. #{block} 0:5:. 5:4:..\n", coll.manifest_text)
end
def test_escaping_through_normalization
coll = Arv::Collection.new(MANY_ESCAPES_MANIFEST)
- coll.import_manifest!(MANY_ESCAPES_MANIFEST)
+ coll.import_manifest(MANY_ESCAPES_MANIFEST)
# The result should simply duplicate the file spec.
# The source file spec has an unescaped backslash in it.
# It's OK for the Collection class to properly escape that.
@@ -81,7 +81,7 @@ class CollectionTest < Minitest::Test
blocks = random_blocks(2, 9)
coll = Arv::Collection.new
blocks.each do |block|
- coll.import_manifest!(". #{block} 1:8:#{file_name}\n")
+ coll.import_manifest(". #{block} 1:8:#{file_name}\n")
end
assert_equal(". #{blocks.join(' ')} 1:8:#{out_name} 10:8:#{out_name}\n",
coll.manifest_text)
@@ -94,7 +94,7 @@ class CollectionTest < Minitest::Test
def test_concatenation_with_locator_overlap(over_index=0)
blocks = random_blocks(4, 2)
coll = Arv::Collection.new(". #{blocks.join(' ')} 0:8:file\n")
- coll.import_manifest!(". #{blocks[over_index, 2].join(' ')} 0:4:file\n")
+ coll.import_manifest(". #{blocks[over_index, 2].join(' ')} 0:4:file\n")
assert_equal(". #{blocks.join(' ')} 0:8:file #{over_index * 2}:4:file\n",
coll.manifest_text)
end
@@ -110,32 +110,78 @@ class CollectionTest < Minitest::Test
def test_concatenation_with_partial_locator_overlap
blocks = random_blocks(3, 3)
coll = Arv::Collection.new(". #{blocks[0, 2].join(' ')} 0:6:overlap\n")
- coll.import_manifest!(". #{blocks[1, 2].join(' ')} 0:6:overlap\n")
+ coll.import_manifest(". #{blocks[1, 2].join(' ')} 0:6:overlap\n")
assert_equal(". #{blocks.join(' ')} 0:6:overlap 3:6:overlap\n",
coll.manifest_text)
end
- ### .normalize!
+ ### .normalize
def test_normalize
block = random_block
coll = Arv::Collection.new(". #{block} 0:0:f2 0:0:f1\n")
- coll.normalize!
+ coll.normalize
assert_equal(". #{block} 0:0:f1 0:0:f2\n", coll.manifest_text)
end
- ### .copy!
+ def test_normalization_file_spans_two_whole_blocks(file_specs="0:10:f1",
+ num_blocks=2)
+ blocks = random_blocks(num_blocks, 5)
+ m_text = ". #{blocks.join(' ')} #{file_specs}\n"
+ coll = Arv::Collection.new(m_text.dup)
+ coll.normalize
+ assert_equal(m_text, coll.manifest_text)
+ end
+
+ def test_normalization_file_fits_beginning_block
+ test_normalization_file_spans_two_whole_blocks("0:7:f1")
+ end
+
+ def test_normalization_file_fits_end_block
+ test_normalization_file_spans_two_whole_blocks("3:7:f1")
+ end
+
+ def test_normalization_file_spans_middle
+ test_normalization_file_spans_two_whole_blocks("3:5:f1")
+ end
+
+ def test_normalization_file_spans_three_whole_blocks
+ test_normalization_file_spans_two_whole_blocks("0:15:f1", 3)
+ end
+
+ def test_normalization_file_skips_bytes
+ test_normalization_file_spans_two_whole_blocks("0:3:f1 5:5:f1")
+ end
+
+ def test_normalization_file_inserts_bytes
+ test_normalization_file_spans_two_whole_blocks("0:3:f1 5:3:f1 3:2:f1")
+ end
+
+ def test_normalization_file_duplicates_bytes
+ test_normalization_file_spans_two_whole_blocks("2:3:f1 2:3:f1", 1)
+ end
+
+ def test_normalization_dedups_locators
+ blocks = random_blocks(2, 5)
+ coll = Arv::Collection.new(". %s %s 1:8:f1 11:8:f1\n" %
+ [blocks.join(" "), blocks.reverse.join(" ")])
+ coll.normalize
+ assert_equal(". #{blocks.join(' ')} 1:8:f1 6:4:f1 0:4:f1\n",
+ coll.manifest_text)
+ end
+
+ ### .cp_r
def test_simple_file_copy
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.copy!("./simple.txt", "./new")
+ coll.cp_r("./simple.txt", "./new")
assert_equal(SIMPLEST_MANIFEST.sub(" 0:9:", " 0:9:new 0:9:"),
coll.manifest_text)
end
def test_copy_file_into_other_stream(target="./s1/f2", basename="f2")
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.copy!("./f2", target)
+ coll.cp_r("./f2", target)
expected = "%s./s1 %s 0:5:f1 14:4:%s 5:4:f3\n" %
[TWO_BY_TWO_MANIFEST_A.first,
TWO_BY_TWO_BLOCKS.reverse.join(" "), basename]
@@ -152,7 +198,7 @@ class CollectionTest < Minitest::Test
def test_copy_file_over_in_other_stream(target="./s1/f1")
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.copy!("./f1", target)
+ coll.cp_r("./f1", target)
expected = "%s./s1 %s 0:5:f1 14:4:f3\n" %
[TWO_BY_TWO_MANIFEST_A.first, TWO_BY_TWO_BLOCKS.join(" ")]
assert_equal(expected, coll.manifest_text)
@@ -164,7 +210,7 @@ class CollectionTest < Minitest::Test
def test_simple_stream_copy
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.copy!("./s1", "./sNew")
+ coll.cp_r("./s1", "./sNew")
new_line = TWO_BY_TWO_MANIFEST_A.last.sub("./s1 ", "./sNew ")
assert_equal(TWO_BY_TWO_MANIFEST_S + new_line, coll.manifest_text)
end
@@ -172,7 +218,7 @@ class CollectionTest < Minitest::Test
def test_copy_stream_into_other_stream(target="./dir2/subdir",
basename="subdir")
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- coll.copy!("./dir1/subdir", target)
+ coll.cp_r("./dir1/subdir", target)
new_line = MULTILEVEL_MANIFEST.lines[4].sub("./dir1/subdir ",
"./dir2/#{basename} ")
assert_equal(MULTILEVEL_MANIFEST + new_line, coll.manifest_text)
@@ -189,9 +235,9 @@ class CollectionTest < Minitest::Test
def test_copy_stream_over_empty_stream
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
(1..3).each do |file_num|
- coll.remove!("./dir0/subdir/file#{file_num}")
+ coll.rm("./dir0/subdir/file#{file_num}")
end
- coll.copy!("./dir1/subdir", "./dir0")
+ coll.cp_r("./dir1/subdir", "./dir0")
expected = MULTILEVEL_MANIFEST.lines
expected[2] = expected[4].sub("./dir1/", "./dir0/")
assert_equal(expected.join(""), coll.manifest_text)
@@ -200,7 +246,7 @@ class CollectionTest < Minitest::Test
def test_copy_stream_over_file_raises_ENOTDIR
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
assert_raises(Errno::ENOTDIR) do
- coll.copy!("./s1", "./f2")
+ coll.cp_r("./s1", "./f2")
end
end
@@ -211,7 +257,7 @@ class CollectionTest < Minitest::Test
"./zdir #{blocks[1]} 0:9:zfile\n",
"./zdir/subdir #{blocks[2]} 0:1:s2 1:2:zero\n"]
coll = Arv::Collection.new(manifest_a.join(""))
- coll.copy!("./subdir", "./zdir")
+ coll.cp_r("./subdir", "./zdir")
manifest_a[2] = "./zdir/subdir %s %s 0:1:s1 9:1:s2 1:2:zero\n" %
[blocks[0], blocks[2]]
assert_equal(manifest_a.join(""), coll.manifest_text)
@@ -220,7 +266,7 @@ class CollectionTest < Minitest::Test
def test_copy_stream_into_substream(source="./dir1",
target="./dir1/subdir/dir1")
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- coll.copy!(source, target)
+ coll.cp_r(source, target)
expected = MULTILEVEL_MANIFEST.lines.flat_map do |line|
[line, line.gsub(/^#{Regexp.escape(source)}([\/ ])/, "#{target}\\1")].uniq
end
@@ -233,8 +279,8 @@ class CollectionTest < Minitest::Test
def test_adding_to_root_after_copy
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.copy!(".", "./root")
- coll.import_manifest!(COLON_FILENAME_MANIFEST)
+ coll.cp_r(".", "./root")
+ coll.import_manifest(COLON_FILENAME_MANIFEST)
got_lines = coll.manifest_text.lines
assert_equal(2, got_lines.size)
assert_match(/^\. \S{33,} \S{33,} 0:9:file:test\.txt 9:9:simple\.txt\n/,
@@ -244,7 +290,7 @@ class CollectionTest < Minitest::Test
def test_copy_chaining
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.copy!("./simple.txt", "./a").copy!("./a", "./b")
+ coll.cp_r("./simple.txt", "./a").cp_r("./a", "./b")
assert_equal(SIMPLEST_MANIFEST.sub(" 0:9:", " 0:9:a 0:9:b 0:9:"),
coll.manifest_text)
end
@@ -261,7 +307,7 @@ class CollectionTest < Minitest::Test
def test_copy_file_from_other_collection(src_stream=".", dst_stream="./s1")
blocks, src_text, dst_text, src_coll, dst_coll =
prep_two_collections_for_copy(src_stream, dst_stream)
- dst_coll.copy!("#{src_stream}/f1", dst_stream, src_coll)
+ dst_coll.cp_r("#{src_stream}/f1", dst_stream, src_coll)
assert_equal("#{dst_stream} #{blocks.join(' ')} 0:8:f1 8:8:f2\n",
dst_coll.manifest_text)
assert_equal(src_text, src_coll.manifest_text)
@@ -274,7 +320,7 @@ class CollectionTest < Minitest::Test
def test_copy_stream_from_other_collection
blocks, src_text, dst_text, src_coll, dst_coll =
prep_two_collections_for_copy("./s2", "./s1")
- dst_coll.copy!("./s2", "./s1", src_coll)
+ dst_coll.cp_r("./s2", "./s1", src_coll)
assert_equal(dst_text + src_text.sub("./s2 ", "./s1/s2 "),
dst_coll.manifest_text)
assert_equal(src_text, src_coll.manifest_text)
@@ -283,7 +329,7 @@ class CollectionTest < Minitest::Test
def test_copy_stream_from_other_collection_to_root
blocks, src_text, dst_text, src_coll, dst_coll =
prep_two_collections_for_copy("./s1", ".")
- dst_coll.copy!("./s1", ".", src_coll)
+ dst_coll.cp_r("./s1", ".", src_coll)
assert_equal(dst_text + src_text, dst_coll.manifest_text)
assert_equal(src_text, src_coll.manifest_text)
end
@@ -291,7 +337,7 @@ class CollectionTest < Minitest::Test
def test_copy_empty_source_path_raises_ArgumentError(src="", dst="./s1")
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
assert_raises(ArgumentError) do
- coll.copy!(src, dst)
+ coll.cp_r(src, dst)
end
end
@@ -299,18 +345,18 @@ class CollectionTest < Minitest::Test
test_copy_empty_source_path_raises_ArgumentError(".", "")
end
- ### .rename!
+ ### .rename
def test_simple_file_rename
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.rename!("./simple.txt", "./new")
+ coll.rename("./simple.txt", "./new")
assert_equal(SIMPLEST_MANIFEST.sub(":simple.txt", ":new"),
coll.manifest_text)
end
def test_rename_file_into_other_stream(target="./s1/f2", basename="f2")
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.rename!("./f2", target)
+ coll.rename("./f2", target)
expected = ". %s 0:5:f1\n./s1 %s 0:5:f1 14:4:%s 5:4:f3\n" %
[TWO_BY_TWO_BLOCKS.first,
TWO_BY_TWO_BLOCKS.reverse.join(" "), basename]
@@ -327,7 +373,7 @@ class CollectionTest < Minitest::Test
def test_rename_file_over_in_other_stream(target="./s1/f1")
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.rename!("./f1", target)
+ coll.rename("./f1", target)
expected = ". %s 5:4:f2\n./s1 %s 0:5:f1 14:4:f3\n" %
[TWO_BY_TWO_BLOCKS.first, TWO_BY_TWO_BLOCKS.join(" ")]
assert_equal(expected, coll.manifest_text)
@@ -339,7 +385,7 @@ class CollectionTest < Minitest::Test
def test_simple_stream_rename
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
- coll.rename!("./s1", "./newS")
+ coll.rename("./s1", "./newS")
assert_equal(TWO_BY_TWO_MANIFEST_S.sub("\n./s1 ", "\n./newS "),
coll.manifest_text)
end
@@ -347,7 +393,7 @@ class CollectionTest < Minitest::Test
def test_rename_stream_into_other_stream(target="./dir2/subdir",
basename="subdir")
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- coll.rename!("./dir1/subdir", target)
+ coll.rename("./dir1/subdir", target)
expected = MULTILEVEL_MANIFEST.lines
replaced_line = expected.delete_at(4)
expected << replaced_line.sub("./dir1/subdir ", "./dir2/#{basename} ")
@@ -365,9 +411,9 @@ class CollectionTest < Minitest::Test
def test_rename_stream_over_empty_stream
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
(1..3).each do |file_num|
- coll.remove!("./dir0/subdir/file#{file_num}")
+ coll.rm("./dir0/subdir/file#{file_num}")
end
- coll.rename!("./dir1/subdir", "./dir0")
+ coll.rename("./dir1/subdir", "./dir0")
expected = MULTILEVEL_MANIFEST.lines
expected[2] = expected.delete_at(4).sub("./dir1/", "./dir0/")
assert_equal(expected.sort.join(""), coll.manifest_text)
@@ -376,21 +422,21 @@ class CollectionTest < Minitest::Test
def test_rename_stream_over_file_raises_ENOTDIR
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
assert_raises(Errno::ENOTDIR) do
- coll.rename!("./s1", "./f2")
+ coll.rename("./s1", "./f2")
end
end
def test_rename_stream_over_nonempty_stream_raises_ENOTEMPTY
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
assert_raises(Errno::ENOTEMPTY) do
- coll.rename!("./dir1/subdir", "./dir0")
+ coll.rename("./dir1/subdir", "./dir0")
end
end
def test_rename_stream_into_substream(source="./dir1",
target="./dir1/subdir/dir1")
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- coll.rename!(source, target)
+ coll.rename(source, target)
assert_equal(MULTILEVEL_MANIFEST.gsub(/^#{Regexp.escape(source)}([\/ ])/m,
"#{target}\\1"),
coll.manifest_text)
@@ -402,23 +448,23 @@ class CollectionTest < Minitest::Test
def test_adding_to_root_after_rename
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.rename!(".", "./root")
- coll.import_manifest!(SIMPLEST_MANIFEST)
+ coll.rename(".", "./root")
+ coll.import_manifest(SIMPLEST_MANIFEST)
assert_equal(SIMPLEST_MANIFEST + SIMPLEST_MANIFEST.sub(". ", "./root "),
coll.manifest_text)
end
def test_rename_chaining
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.rename!("./simple.txt", "./x").rename!("./x", "./simple.txt")
+ coll.rename("./simple.txt", "./x").rename("./x", "./simple.txt")
assert_equal(SIMPLEST_MANIFEST, coll.manifest_text)
end
- ### .remove!
+ ### .rm
def test_simple_remove
coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S.dup)
- coll.remove!("./f2")
+ coll.rm("./f2")
assert_equal(TWO_BY_TWO_MANIFEST_S.sub(" 5:4:f2", ""), coll.manifest_text)
end
@@ -430,79 +476,142 @@ class CollectionTest < Minitest::Test
def test_remove_all_files_in_substream
empty_stream_and_assert do |coll|
- coll.remove!("./s1/f1")
- coll.remove!("./s1/f3")
+ coll.rm("./s1/f1")
+ coll.rm("./s1/f3")
end
end
def test_remove_all_files_in_root_stream
empty_stream_and_assert(1) do |coll|
- coll.remove!("./f1")
- coll.remove!("./f2")
+ coll.rm("./f1")
+ coll.rm("./f2")
end
end
- def test_remove_empty_stream
+ def test_chaining_removes
empty_stream_and_assert do |coll|
- coll.remove!("./s1/f1")
- coll.remove!("./s1/f3")
- coll.remove!("./s1")
+ coll.rm("./s1/f1").rm("./s1/f3")
end
end
- def test_recursive_remove
- empty_stream_and_assert do |coll|
- coll.remove!("./s1", recursive: true)
+ def test_remove_last_file
+ coll = Arv::Collection.new(SIMPLEST_MANIFEST)
+ coll.rm("./simple.txt")
+ assert_equal("", coll.manifest_text)
+ end
+
+ def test_remove_nonexistent_file_raises_ENOENT(path="./NoSuchFile",
+ method=:rm)
+ coll = Arv::Collection.new(SIMPLEST_MANIFEST)
+ assert_raises(Errno::ENOENT) do
+ coll.send(method, path)
end
end
- def test_recursive_remove_on_files
- empty_stream_and_assert do |coll|
- coll.remove!("./s1/f1", recursive: true)
- coll.remove!("./s1/f3", recursive: true)
+ def test_remove_from_nonexistent_stream_raises_ENOENT
+ test_remove_nonexistent_file_raises_ENOENT("./NoSuchStream/simple.txt")
+ end
+
+ def test_remove_stream_raises_EISDIR(path="./s1")
+ coll = Arv::Collection.new(TWO_BY_TWO_MANIFEST_S)
+ assert_raises(Errno::EISDIR) do
+ coll.rm(path)
end
end
- def test_chaining_removes
+ def test_remove_root_raises_EISDIR
+ test_remove_stream_raises_EISDIR(".")
+ end
+
+ def test_remove_empty_string_raises_ArgumentError
+ coll = Arv::Collection.new(SIMPLEST_MANIFEST)
+ assert_raises(ArgumentError) do
+ coll.rm("")
+ end
+ end
+
+ ### rm_r
+
+ def test_recursive_remove
empty_stream_and_assert do |coll|
- coll.remove!("./s1/f1").remove!("./s1/f3")
+ coll.rm_r("./s1")
end
end
- def test_remove_last_file
- coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- coll.remove!("./simple.txt")
- assert_equal("", coll.manifest_text)
+ def test_recursive_remove_on_files
+ empty_stream_and_assert do |coll|
+ coll.rm_r("./s1/f1")
+ coll.rm_r("./s1/f3")
+ end
end
- def test_remove_root_stream
+ def test_recursive_remove_root
coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- coll.remove!(".", recursive: true)
+ coll.rm_r(".")
assert_equal("", coll.manifest_text)
end
- def test_remove_nonexistent_file_raises_ENOENT(path="./NoSuchFile")
+ def test_rm_r_nonexistent_file_raises_ENOENT(path="./NoSuchFile")
+ test_remove_nonexistent_file_raises_ENOENT("./NoSuchFile", :rm_r)
+ end
+
+ def test_rm_r_from_nonexistent_stream_raises_ENOENT
+ test_remove_nonexistent_file_raises_ENOENT("./NoSuchStream/file", :rm_r)
+ end
+
+ def test_rm_r_empty_string_raises_ArgumentError
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- assert_raises(Errno::ENOENT) do
- coll.remove!(path)
+ assert_raises(ArgumentError) do
+ coll.rm_r("")
end
end
- def test_remove_from_nonexistent_stream_raises_ENOENT
- test_remove_nonexistent_file_raises_ENOENT("./NoSuchStream/simple.txt")
+ ### .modified?
+
+ def test_new_collection_unmodified(*args)
+ coll = Arv::Collection.new(*args)
+ yield coll if block_given?
+ refute(coll.modified?)
end
- def test_remove_nonempty_stream_raises_ENOTEMPTY
- coll = Arv::Collection.new(MULTILEVEL_MANIFEST)
- assert_raises(Errno::ENOTEMPTY) do
- coll.remove!("./dir1/subdir")
+ def test_collection_unmodified_after_instantiation
+ test_new_collection_unmodified(SIMPLEST_MANIFEST)
+ end
+
+ def test_collection_unmodified_after_initial_import
+ test_new_collection_unmodified do |coll|
+ coll.import_manifest(SIMPLEST_MANIFEST)
end
end
- def test_remove_empty_string_raises_ArgumentError
+ def test_collection_unmodified_after_mark
+ test_new_collection_unmodified(SIMPLEST_MANIFEST) do |coll|
+ coll.cp_r("./simple.txt", "./copy")
+ coll.unmodified
+ end
+ end
+
+ def check_collection_modified
coll = Arv::Collection.new(SIMPLEST_MANIFEST)
- assert_raises(ArgumentError) do
- coll.remove!("")
+ yield coll
+ assert(coll.modified?)
+ end
+
+ def test_collection_modified_after_copy
+ check_collection_modified do |coll|
+ coll.cp_r("./simple.txt", "./copy")
+ end
+ end
+
+ def test_collection_modified_after_remove
+ check_collection_modified do |coll|
+ coll.rm("./simple.txt")
+ end
+ end
+
+ def test_collection_modified_after_rename
+ check_collection_modified do |coll|
+ coll.rename("./simple.txt", "./newname")
end
end
end
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list