Skip to content

Commit

Permalink
Check for path errors eagerly during load/save (#263) (#265)
Browse files Browse the repository at this point in the history
* add missing gzip test file (and parent csv file)

* add eager checks for file and path existence

* create dummy file for multierr test

* add tests for path errors

* mkpath eagerly during save if dir doesn't exist
  • Loading branch information
IanButterworth authored Jul 17, 2020
1 parent 882dc75 commit c106b1f
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 1 deletion.
7 changes: 7 additions & 0 deletions src/loadsave.jl
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ for fn in (:load, :loadstreaming, :metadata)
isfile(filename(q)) || open(filename(q)) # force systemerror
throw(UnknownFormat(q))
end
if q isa File
!isfile(filename(q)) && throw(ArgumentError("No file exists at given path: $(filename(q))"))
end
libraries = applicable_loaders(q)
failures = Any[]
for library in libraries
Expand All @@ -191,6 +194,10 @@ for fn in (:save, :savestreaming)
gen2_func_name = Symbol("fileio_", fn)
@eval function $fn(@nospecialize(q::Formatted), @nospecialize(data...); @nospecialize(options...))
unknown(q) && throw(UnknownFormat(q))
if q isa File
isdir(filename(q)) && throw(ArgumentError("Given file path is a directory: $(filename(q))"))
!isdir(dirname(filename(q))) && mkpath(dirname(filename(q)))
end
libraries = applicable_savers(q)
failures = Any[]
for library in libraries
Expand Down
28 changes: 27 additions & 1 deletion test/error_handling.jl
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
println("these tests will print warnings: ")

module PathError
import FileIO: File, @format_str
save(file::File{format"PATHERROR"}, data) = nothing
load(file::File{format"PATHERROR"}) = nothing
end
add_format(format"PATHERROR", (), ".patherror", [:PathError])

@testset "Path errors" begin
# handling a nonexistent parent directory, during save
temp_dir = joinpath(mktempdir(), "dir_that_did_not_exist")
@assert !isdir(temp_dir) "Testing error. This dir shouldn't exist"
fn = joinpath(temp_dir, "file.patherror")
save(fn, "test content")
@test isdir(temp_dir)

# handling a filepath that's an existing directory, during save
@test_throws ArgumentError save(format"PATHERROR", mktempdir(), "test content")

# handling a nonexistent filepath, during load
@test_throws ArgumentError load(joinpath(mktempdir(), "dummy.patherror"))
end

@testset "Not installed" begin
add_format(format"NotInstalled", (), ".not_installed", [:NotInstalled])
@test_throws ArgumentError save("test.not_installed", nothing)
Expand Down Expand Up @@ -60,6 +82,10 @@ end
[:MultiError1],
[:MultiError2]
)
ret = @test_throws ErrorException load("test.multierr")
tmpfile = joinpath(mktempdir(), "test.multierr")
open(tmpfile, "w") do io
println(io, "dummy content")
end
ret = @test_throws ErrorException load(tmpfile)
#@test ret.value.msg == "1" # this is 0.5 only
end
2 changes: 2 additions & 0 deletions test/files/file.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
dummy,data
0,1
Binary file added test/files/file.csv.gz
Binary file not shown.

0 comments on commit c106b1f

Please sign in to comment.