Skip to content

Commit

Permalink
use ArgTools for read/write arguments, refactor API tests (#44)
Browse files Browse the repository at this point in the history 
The API testing takes a long time now since we're testing 3 formats
and 5 different types for each argument for up to 3 arguments, so
a total 375 combinations, which is some serious overkill. Refactoring
made it somewhat faster (and clearer), but could still be cut down.
  • Loading branch information
@StefanKarpinski
StefanKarpinski authored Jun 8, 2020
1 parent 0dc0e70 commit d8512d8
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 121 deletions.
1 change: 1 addition & 0 deletions Project.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ authors = ["Stefan Karpinski <[email protected]>"]
version = "1.1.1"

[deps]
ArgTools = "0dad84c5-d112-42e6-8d28-ef12dabb789f"
BufferedStreams = "e1450e63-4bb3-523b-b2a4-4ffa8c0fd77d"
CodecBzip2 = "523fee87-0ab8-5b00-afb7-3ecf72e48cfd"
SuffixArrays = "24f65c1e-0a10-5d3d-8a1f-a83399f3fced"
Expand Down
74 changes: 23 additions & 51 deletions src/BSDiff.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module BSDiff

export bsdiff, bspatch, bsindex

using ArgTools
using SuffixArrays
using TranscodingStreams, CodecBzip2
using TranscodingStreams: Codec
Expand Down Expand Up @@ -58,35 +59,6 @@ detect_format(path::AbstractString) = open(detect_format, path)

const INDEX_HEADER = "SUFFIX ARRAY\0"

## some API utilities for arguments ##

open_read(f::Function, file::AbstractString) = open(f, file)
open_read(f::Function, file::IO) = f(file)

function open_write(f::Function, file::AbstractString)
try open(f, file, write=true)
catch
rm(file, force=true)
rethrow()
end
return file
end
function open_write(f::Function, file::Nothing)
file, io = mktemp()
try f(io)
catch
close(io)
rm(file, force=true)
rethrow()
end
close(io)
return file
end
function open_write(f::Function, file::IO)
f(file)
return file
end

## high-level API (similar to the C tool) ##

"""
Expand All @@ -109,15 +81,15 @@ patch format. The classic patch format is generated by default, but the Endsley
format can be selected with `bsdiff(old, new, patch, format = :endsley)`.
"""
function bsdiff(
old::Union{AbstractString, IO, NTuple{2, Union{AbstractString, IO}}},
new::Union{AbstractString, IO},
patch::Union{AbstractString, IO, Nothing} = nothing;
old::Union{ArgRead, NTuple{2, ArgRead}},
new::ArgRead,
patch::Union{ArgWrite, Nothing} = nothing;
format::Symbol = DEFAULT_FORMAT,
)
type = patch_type(format)
old_data, index = data_and_index(old)
new_data = open_read(read, new)
open_write(patch) do patch_io
new_data = arg_read(read, new)
arg_write(patch) do patch_io
write(patch_io, format_magic(type))
patch_obj = write_start(type, patch_io, old_data, new_data)
generate_patch(patch_obj, old_data, new_data, index)
Expand All @@ -143,22 +115,22 @@ that will be accepted, however, you can use this keyword argument: `bspatch`
will raise an error unless the patch file has indicated format.
"""
function bspatch(
old::Union{AbstractString, IO},
new::Union{AbstractString, IO, Nothing},
patch::Union{AbstractString, IO};
old::ArgRead,
new::Union{ArgWrite, Nothing},
patch::ArgRead;
format::Symbol = :auto,
)
format == :auto || format in keys(FORMATS) ||
error("unknown patch format: $format")
old_data = open_read(read, old)
open_read(patch) do patch_io
old_data = arg_read(read, old)
arg_read(patch) do patch_io
detected = detect_format(patch_io)
detected == :unknown && error("unrecognized/corrupt patch file")
format == :auto || format == detected ||
error("patch has $detected format, expected $format format")
type = patch_type(detected)
patch_obj = read_start(type, patch_io)
open_write(new) do new_io
arg_write(new) do new_io
new_io = BufferedOutputStream(new_io)
apply_patch(patch_obj, old_data, new_io)
finalize(patch_obj)
Expand All @@ -168,8 +140,8 @@ function bspatch(
end

function bspatch(
old::Union{AbstractString, IO},
patch::Union{AbstractString, IO};
old::ArgRead,
patch::ArgRead;
format::Symbol = :auto,
)
bspatch(old, nothing, patch; format = format)
Expand All @@ -189,11 +161,11 @@ it can significantly speed up generting diffs from the same old file to multiple
different new files.
"""
function bsindex(
old::Union{AbstractString, IO},
index::Union{AbstractString, IO, Nothing} = nothing,
old::ArgRead,
index::Union{ArgWrite, Nothing} = nothing,
)
old_data = open_read(read, old)
open_write(index) do index_io
old_data = arg_read(read, old)
arg_write(index) do index_io
write(index_io, INDEX_HEADER)
index_data = generate_index(old_data)
write(index_io, UInt8(sizeof(eltype(index_data))))
Expand All @@ -205,16 +177,16 @@ end

const IndexType{T<:Integer} = Vector{T}

function data_and_index(data_path::Union{AbstractString, IO})
data = open_read(read, data_path)
function data_and_index(data_path::ArgRead)
data = arg_read(read, data_path)
data, generate_index(data)
end

function data_and_index(
(data_path, index_path)::NTuple{2, Union{AbstractString, IO}},
(data_path, index_path)::NTuple{2, ArgRead},
)
data = open_read(read, data_path)
index = open_read(index_path) do index_io
data = arg_read(read, data_path)
index = arg_read(index_path) do index_io
hdr = String(read(index_io, ncodeunits(INDEX_HEADER)))
hdr == INDEX_HEADER || error("corrupt bsdiff index file")
unit = Int(read(index_io, UInt8))
Expand Down
158 changes: 88 additions & 70 deletions test/runtests.jl
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using Test
using BSDiff
using ArgTools
using Pkg.Artifacts

import bsdiff_classic_jll
Expand All @@ -9,96 +10,110 @@ import zrl_jll
const test_data = artifact"test_data"
const FORMATS = sort!(collect(keys(BSDiff.FORMATS)))

readers(file) = [
f -> f(file),
f -> open(f, file),
f -> open(f, `cat $file`),
]
writers(file) = [
f -> f(file),
f -> open(f, file, write=true),
f -> open(f, pipeline(`cat`, file), write=true),
]

@testset "BSDiff" begin
@testset "API coverage" begin
# create new, old and reference patch files
# create new and old files
dir = mktempdir()
old_file = joinpath(dir, "old")
new_file = joinpath(dir, "new")
index_file = joinpath(dir, "index")
write(old_file, "Goodbye, world.")
write(new_file, "Hello, world!")
for format in (nothing, :classic, :endsley)
fmt = format == nothing ? [] : [:format => format]
# check API passing only two paths
@testset "2-arg API" begin
for old in readers(old_file),
new in readers(new_file)
patch_file = old() do old; new() do new
bsdiff(old, new; fmt...)
end; end
for patch in readers(patch_file)
new_file′ = old() do old; patch() do patch
bspatch(old, patch; fmt...)
end; end
@test read(new_file′, String) == "Hello, world!"
new_file′ = old() do old; patch() do patch
bspatch(old, patch) # format auto-detected
end; end
@test read(new_file′, String) == "Hello, world!"

# generate reference index
index_file = joinpath(dir, "index")
index_file = bsindex(old_file)

@testset "bsindex" begin
arg_readers(old_file) do old
index_file′ = @arg_test old bsindex(old)
@test read(index_file′) == read(index_file)

arg_writers() do index_file′, index
@arg_test old index begin
@test index == bsindex(old, index)
end
@test read(index_file′) == read(index_file)
end
end
# check API passing all three paths
@testset "3-arg API" begin
patch_file = joinpath(dir, "patch")
new_file′ = joinpath(dir, "new′")
for old in readers(old_file),
new in readers(new_file),
patch in writers(patch_file)
old() do old; new() do new; patch() do patch
bsdiff(old, new, patch; fmt...)
end; end; end
for new′ in writers(new_file′),
patch in readers(patch_file)
old() do old; new′() do new′; patch() do patch
bspatch(old, new′, patch; fmt...)
end; end; end
@test read(new_file′, String) == "Hello, world!"
old() do old; new′() do new′; patch() do patch
bspatch(old, new′, patch) # format auto-detected
end; end; end
@test read(new_file′, String) == "Hello, world!"
end

for format in (nothing, :classic, :endsley)
fmt = format == nothing ? [] : [:format => format]

# generate and test reference patch first
patch_file = bsdiff(old_file, new_file; fmt...)
@testset "reference patch" begin
new_file′ = bspatch(old_file, patch_file; fmt...)
@test read(new_file′) == read(new_file)
rm(new_file′)
end

@testset "bsdiff" begin
arg_readers(old_file) do old
arg_readers(new_file) do new
patch_file′ = @arg_test old new begin
bsdiff(old, new; fmt...)
end
@test read(patch_file′) == read(patch_file)
rm(patch_file′)

arg_readers(index_file) do index
patch_file′ = @arg_test old index new begin
bsdiff((old, index), new; fmt...)
end
@test read(patch_file′) == read(patch_file)
rm(patch_file′)
end

arg_writers() do patch_file′, patch
@arg_test old new patch begin
@test patch == bsdiff(old, new, patch; fmt...)
end
@test read(patch_file′) == read(patch_file)
end
end
end
end
@testset "bsindex API" begin
for old in readers(old_file),
index in writers(index_file)
old() do old; index() do index
bsindex(old, index)
end; end
for index in readers(index_file),
new in readers(new_file)
patch_file = old() do old; index() do index; new() do new
bsdiff((old, index), new; fmt...)
end; end; end
new_file′ = old() do old
bspatch(old, patch_file; fmt...)

@testset "bspatch" begin
arg_readers(old_file) do old
arg_readers(patch_file) do patch
# auto-detected format
new_file′ = @arg_test old patch begin
bspatch(old, patch)
end
@test read(new_file′) == read(new_file)
rm(new_file′)

arg_writers() do new_file′, new
@arg_test old new patch begin
@test new == bspatch(old, new, patch)
end
@test read(new_file′) == read(new_file)
end
format === nothing &&
return # tests below are the same

# explicit format
new_file′ = @arg_test old patch begin
bspatch(old, patch; fmt...) # explicit format
end
@test read(new_file′) == read(new_file)
rm(new_file′)

arg_writers() do new_file′, new
@arg_test old new patch begin
@test new == bspatch(old, new, patch; fmt...)
end
@test read(new_file′) == read(new_file)
end
@test read(new_file′, String) == "Hello, world!"
end
# test that 1-arg API makes the same file
index_file′ = old() do old
bsindex(old)
end
@test read(index_file) == read(index_file′)
end
end
end
rm(dir, recursive=true, force=true)
end

@testset "registry data" begin
registry_data = joinpath(test_data, "registry")
old = joinpath(registry_data, "before.tar")
Expand All @@ -117,6 +132,7 @@ writers(file) = [
@test read(new) == read(new′)
@show filesize(patch)
end

@testset "ZRL data (w/timing)" for format in FORMATS
println("[ ZRL data ]")
@show format
Expand All @@ -136,6 +152,7 @@ writers(file) = [
rm(old_zrl)
rm(new_zrl)
end

@testset "low-level API" begin
# test that diff is identical to reference diff
index = BSDiff.generate_index(old_data)
Expand All @@ -153,6 +170,7 @@ writers(file) = [
end
@test new_data == new_data′
end

if !Sys.iswindows() # bsdiff JLLs don't compile on Windows
for (format, jll) in [
(:classic, bsdiff_classic_jll),
Expand Down

2 comments on commit d8512d8

@StefanKarpinski
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JuliaRegistrator register

@JuliaRegistrator
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error while trying to register: "Tag with name v1.1.1 already exists and points to a different commit"

Please sign in to comment.