From 44bbf99ec7cb927ce6c98fbd46c90deea4ad8eff Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Wed, 3 Mar 2021 04:35:12 -0600 Subject: [PATCH] Delete cruft & improve test coverage --- src/error_handling.jl | 34 --------------------------- src/query.jl | 32 ++++---------------------- src/types.jl | 6 +++-- test/query.jl | 53 +++++++++++++++++++++++++++++++++++++++++-- 4 files changed, 59 insertions(+), 66 deletions(-) diff --git a/src/error_handling.jl b/src/error_handling.jl index 4b83aa81..ed3605d9 100644 --- a/src/error_handling.jl +++ b/src/error_handling.jl @@ -24,35 +24,6 @@ Base.showerror(io::IO, e::WriterError) = println( e.msg, "\n Will try next writer." ) -""" -`NotInstalledError` should be thrown when a library is currently not installed. -""" -struct NotInstalledError <: Exception - library::Symbol - message::String -end -Base.showerror(io::IO, e::NotInstalledError) = println(io, e.library, " is not installed.") - -""" -`UnknownFormat` gets thrown when FileIO can't recognize the format of a file. -""" -struct UnknownFormat{T <: Formatted} <: Exception - format::T -end -Base.showerror(io::IO, e::UnknownFormat) = println(io, e.format, " couldn't be recognized by FileIO.") - - -""" -Handles error as soon as they get thrown while doing IO -""" -function handle_current_error(e, library, islast::Bool) - bt = catch_backtrace() - bts = sprint(io->Base.show_backtrace(io, bt)) - message = islast ? "" : "\nTrying next loading library! Please report this issue on the Github page for $library" - @warn string(e, bts, message) -end -handle_current_error(e::NotInstalledError) = @warn string("lib ", e.library, " not installed, trying next library") - struct SpecError <: Exception mod::Module @@ -86,8 +57,3 @@ function handle_exceptions(exceptions::Vector, action) end handle_error(e, q) = throw(e) - -function handle_error(e::NotInstalledError, q) - println("Library \"", e.library, "\" is not installed but is recommended as a library to load format: \"", file_extension(q), "\"") - rethrow(e) -end diff --git a/src/query.jl b/src/query.jl index a4a7c636..73de294c 100644 --- a/src/query.jl +++ b/src/query.jl @@ -76,7 +76,7 @@ function query(filename; checkfile::Bool=true) sym = querysym(filename; checkfile=checkfile) return File{DataFormat{sym}}(filename) end -query(@nospecialize(f::Formatted); checkfile::Bool=true) = f +query(@nospecialize(f::Formatted)) = f # This is recommended for internal use because it returns Symbol (or errors) function querysym(filename; checkfile::Bool=true) @@ -142,9 +142,8 @@ function match(io, @nospecialize(magic::Function)) try magic(io) catch e - println("There was an error in magic function $magic") - println("Please open an issue at FileIO.jl. Error:") - println(e) + @error("""There was an error in magic function $magic. + Please open an issue at FileIO.jl.""", exception=(e, catch_backtrace())) false end end @@ -205,30 +204,7 @@ function query(io::IO, filename = nothing) sym = querysym(io) return Stream{DataFormat{sym}}(io, filename) end -query(io::IO, @nospecialize(filename::Formatted)) = error("no need to query when format is known") - -# TODO?: update to querysym? -function query(io::IO, filename::String, sym::Vector{Symbol}) - pos = position(io) - if seekable(io) - for (f, fmtsym) in magic_func - fmtsym in sym || continue - seek(io, pos) - try - if f(io) - return Stream{DataFormat{fmtsym},typeof(io)}(seek(io, pos), filename) - end - catch e - println("There was an error in magic function $f") - println("Please open an issue at FileIO.jl. Error:") - println(e) - end - end - seek(io, pos) - end - close(io) # FIXME? - nothing -end +query(io::IO, @nospecialize(file::Formatted)) = Stream{DataFormat{formatname(file)::Symbol}}(io, filename(file)) seekable(io::IOBuffer) = io.seekable seekable(::IOStream) = true diff --git a/src/types.jl b/src/types.jl index 4bfc92a5..5c6f5a93 100644 --- a/src/types.jl +++ b/src/types.jl @@ -35,7 +35,7 @@ struct File{F<:DataFormat, Name} <: Formatted{F} filename::Name end File{F}(file::File{F}) where F<:DataFormat = file -File{DataFormat{sym}}(@nospecialize(file::Formatted)) where sym = error("cannot change the format of $file to $sym") +File{DataFormat{sym}}(@nospecialize(file::Formatted)) where sym = throw(ArgumentError("cannot change the format of $file to $sym")) File{F}(file::AbstractString) where F<:DataFormat = File{F,String}(String(file)) # canonicalize to limit type-diversity File{F}(file) where F<:DataFormat = File{F,typeof(file)}(file) @@ -71,9 +71,11 @@ Stream{F,IOtype}(io::IO, filename) where {F<:DataFormat,IOtype} Stream{F,IOtype}(io::IO) where {F<:DataFormat,IOtype} = Stream{F, IOtype}(io, nothing) Stream{F,IOtype}(file::Formatted{F}, io::IO) where {F<:DataFormat,IOtype} = Stream{F,IOtype}(io, filename(file)) -Stream{F,IOtype}(@nospecialize(file::Formatted), io::IO) where {F<:DataFormat,IOtype} = error("cannot change the format of $file to $(formatname(F)::Symbol)") +Stream{F,IOtype}(@nospecialize(file::Formatted), io::IO) where {F<:DataFormat,IOtype} = + throw(ArgumentError("cannot change the format of $file to $(formatname(F)::Symbol)")) Stream{F}(io::IO, args...) where {F<:DataFormat} = Stream{F, typeof(io)}(io, args...) +Stream{F}(file::File, io::IO) where {F<:DataFormat} = Stream{F, typeof(io)}(file, io) Stream(file::File{F}, io::IO) where {F<:DataFormat} = Stream{F}(io, filename(file)) stream(@nospecialize(s::Stream)) = s.io diff --git a/test/query.jl b/test/query.jl index 70b01130..6503c00e 100644 --- a/test/query.jl +++ b/test/query.jl @@ -87,6 +87,7 @@ try @test FileIO.ext2sym[".JNK"] == :JUNK @test FileIO.magic_list == [Pair([0x4a,0x55,0x4e,0x4b],:JUNK)] + add_format(format"OTHER", [0x01, 0x02], ".othr") end @testset "streams" begin @@ -94,11 +95,13 @@ try s = Stream(format"JUNK", io) @test typeof(s) <: Stream{DataFormat{:JUNK},IOBuffer} @test filename(s) == nothing - @test_throws Exception FileIO.file!(s) + @test_throws ErrorException("filename unknown") FileIO.file!(s) s = Stream(format"JUNK", io, "junk.jnk") @test filename(s) == "junk.jnk" s = Stream(format"JUNK", io, "junk2.jnk") @test filename(s) == "junk2.jnk" + s = Stream(format"JUNK", io, "somefile.jnk") + @test FileIO.file!(s) isa File{format"JUNK"} end @testset "query" begin @@ -123,6 +126,20 @@ try @test typeof(q) <: Stream{format"JUNK",typeof(io)} @test !(unknown(q)) @test file_extension(q) == nothing + # unseekable IO + seek(io, 0) + io.seekable = false + @test !FileIO.seekable(io) + q = query(io) + @test typeof(q) <: Stream{format"JUNK",typeof(io)} + io.seekable = true + # too short to match + io2 = IOBuffer() + write(io2, "JU") + seek(io2, 0) + io2.seekable = false + q = query(io2) + @test unknown(q) # File with correct extension str = String(take!(io)) @@ -133,6 +150,17 @@ try q = query(fn) @test typeof(q) <: File{format"JUNK"} @test file_extension(q) == ".jnk" + # for good measure, test some constructors & other query calls + @test query(q) == q + @test File{format"JUNK"}(q) == q + @test_throws ArgumentError("cannot change the format of $q to OTHER") File{format"OTHER"}(q) + open(fn) do io + @test query(io) isa Stream{format"JUNK", typeof(io)} + @test query(io, q) isa Stream{format"JUNK", typeof(io)} + @test Stream(q, io) isa Stream{format"JUNK", typeof(io)} + @test Stream{format"JUNK"}(q, io) isa Stream{format"JUNK", typeof(io)} + @test_throws ArgumentError Stream{format"OTHER"}(q, io) + end rm(fn) @@ -145,6 +173,15 @@ try @test typeof(q) <: File{format"JUNK"} @test file_extension(q) == ".csv" rm(fn) + # erroneous extension with a file that has magic bytes + fn = string(tempname(), ".othr") + open(fn, "w") do file + write(file, str) + end + q = query(fn) + @test typeof(q) <: File{format"JUNK"} + @test query(fn; checkfile=false) isa File{format"OTHER"} + rm(fn) # Format with no magic bytes add_format(format"BAD", (), ".bad") @@ -179,6 +216,15 @@ try @test typeof(q) <: File{format"DOUBLE_1"} rm(fn) + # Busted detection function + busted(io) = error("whoops") + add_format(format"BUSTED", busted, ".bstd") + fn = string(tempname(), ".bstd") + open(fn, "w") do file + write(file, "JUNK stuff") + end + @test (@test_logs (:error,r"There was an error in magic function .*busted") query(fn)) isa File{format"JUNK"} + del_format(format"BUSTED") add_format(format"MAGIC", "this so magic", ".mmm") q = query( "some_non_existant_file.mmm") @@ -214,7 +260,10 @@ try write(file, randstring(19)) # corrupt magic bytes end open(fn, "r") do file - @test_throws Exception skipmagic(file) + @test_throws ErrorException("tried to skip magic bytes of an IO that does not contain the magic bytes of the format. IO: $file") skipmagic(Stream{format"DOUBLE_MAGIC"}(file, fn)) + end + open(fn, "r") do file + @test_throws ErrorException("tried to skip magic bytes of an IO that does not contain the magic bytes of the format. IO: $file") skipmagic(file, format"DOUBLE_MAGIC") end rm(fn) lene0 = length(FileIO.ext2sym)