Fix ENV usage (#305)

* Trim trailing whitespace

* Fix ENV usage

Also fix wrong default of DRWATSON_STOREPATCH in docs.

* Add test for tag!() based on ENV

* Fix CI

Apparently, GitHub CI doesn't expect it's git to perform a commit, so it
doesn't have user.name configured. Let's hope this makes CI happy.

* Add documentation for readenv()

Author: jonas-schulze

src/DrWatson.jl

@@ -7,6 +7,15 @@ const PATH_SEPARATOR = joinpath("_", "_")[2]
 convert_to_kw(ex::Expr) = Expr(:kw,ex.args...)
 convert_to_kw(ex) = error("invalid keyword argument syntax \"$ex\"")
 
+# Misc helpers
+"""
+    readenv(var, default::T) where {T}
+
+Try to read the environment variable `var` and parse it as a `::T`.
+If that fails, return `default`.
+"""
+readenv(var, default::T) where {T} = something(tryparse(T, get(ENV, var, "")), Some(default))
+
 # Pure Julia implementation
 include("project_setup.jl")
 include("naming.jl")
@@ -68,7 +77,7 @@ function __init__()
           It showcases how to eliminate code duplication and streamline your simulation setup
           and run phase using `savename` and `produce_or_load`.
         * By default now `gitpatch` is NOT saved when calling `tag!` and derivative functions.
-          This is due to an unknown problem that causes collecting the git patch to 
+          This is due to an unknown problem that causes collecting the git patch to
           never halt, potentially not saving a user's output.
         \n
         """; color = :light_magenta)

src/saving_files.jl

@@ -24,7 +24,7 @@ end
 
 ## Keywords
 * `suffix = "jld2", prefix = default_prefix(config)` : Used in [`savename`](@ref).
-* `tag::Bool = get(ENV, "DRWATSON_TAG", istaggable(suffix))` : Save the file
+* `tag::Bool = DrWatson.readenv("DRWATSON_TAG", istaggable(suffix))` : Save the file
   using [`tagsave`](@ref) if `true` (which is the default).
 * `gitpath, storepatch` : Given to [`tagsave`](@ref) if `tag` is `true`.
 * `force = false` : If `true` then don't check if file `s` exists and produce
@@ -43,9 +43,9 @@ produce_or_load(f::Function, c; kwargs...) = produce_or_load(c, f; kwargs...)
 produce_or_load(f::Function, path, c; kwargs...) = produce_or_load(path, c, f; kwargs...)
 function produce_or_load(path, c, f::Function;
         suffix = "jld2", prefix = default_prefix(c),
-        tag::Bool = get(ENV, "DRWATSON_TAG", istaggable(suffix)),
+        tag::Bool = readenv("DRWATSON_TAG", istaggable(suffix)),
         gitpath = projectdir(), loadfile = true,
-        storepatch::Bool = get(ENV, "DRWATSON_STOREPATCH", false),
+        storepatch::Bool = readenv("DRWATSON_STOREPATCH", false),
         force = false, verbose = true, wsave_kwargs = Dict(),
         kwargs...
     )
@@ -137,13 +137,13 @@ Keywords `gitpath, storepatch, force,` are propagated to [`tag!`](@ref).
 Any additional keyword arguments are propagated to `wsave`, to e.g.
 enable compression.
 
-The keyword `safe = get(ENV, "DRWATSON_SAFESAVE", false)` decides whether
+The keyword `safe = DrWatson.readenv("DRWATSON_SAFESAVE", false)` decides whether
 to save the file using [`safesave`](@ref).
 """
 function tagsave(file, d;
         gitpath = projectdir(),
-        safe::Bool = get(ENV, "DRWATSON_SAFESAVE", false),
-        storepatch::Bool = get(ENV, "DRWATSON_STOREPATCH", false),
+        safe::Bool = readenv("DRWATSON_SAFESAVE", false),
+        storepatch::Bool = readenv("DRWATSON_STOREPATCH", false),
         force = false, source = nothing, kwargs...
     )
     d2 = tag!(d, gitpath=gitpath, storepatch=storepatch, force=force, source=source)

src/saving_tools.jl

@@ -181,7 +181,7 @@ To restore a repository to the state of a particular model-run do:
 ## Keywords
 * `gitpath = projectdir()`
 * `force = false`
-* `storepatch = get(ENV, "DRWATSON_STOREPATCH", true)`: Whether to collect and store the 
+* `storepatch = DrWatson.readenv("DRWATSON_STOREPATCH", false)`: Whether to collect and store the
   output of [`gitpatch`](@ref) as well.
 
 ## Examples
@@ -198,9 +198,9 @@ Dict{Symbol,Any} with 3 entries:
   :x => 3
 ```
 """
-function tag!(d::AbstractDict{K,T}; 
+function tag!(d::AbstractDict{K,T};
         gitpath = projectdir(), force = false, source = nothing,
-        storepatch::Bool = get(ENV, "DRWATSON_STOREPATCH", false), 
+        storepatch::Bool = readenv("DRWATSON_STOREPATCH", false),
     ) where {K,T}
     @assert (K <: Union{Symbol,String}) "We only know how to tag dictionaries that have keys that are strings or symbols"
     c = gitdescribe(gitpath)

test/stools_tests.jl

@@ -1,6 +1,7 @@
 using DrWatson, Test
 using DataStructures
 using JLD2
+using LibGit2
 
 # Test commit function
 com = gitdescribe(@__DIR__)
@@ -12,30 +13,70 @@ com = gitdescribe(dirname(@__DIR__))
 # TODO: why?
 # @test com[1] == 'v' # test that it has a version tag
 
-# Test isdirty.
-if isdirty(@__DIR__)
-    @test endswith(gitdescribe(@__DIR__), "_dirty")
-else
-    @test !endswith(gitdescribe(@__DIR__), "_dirty")
+# Set up a clean and a dirty repo.
+function _setup_repo(dirty)
+    path = mktempdir(cleanup=true) # delete path on process exit
+    repo = LibGit2.init(path)
+    john = LibGit2.Signature("Dr. John H. Watson", "snail mail only")
+    write(joinpath(path, "foo.txt"), "bar\n")
+    LibGit2.add!(repo, "foo.txt")
+    LibGit2.commit(repo, "tmp repo commit", author=john, committer=john)
+    dirty && write(joinpath(path, "foo.txt"), "baz\n")
+    return path
 end
+dpath = _setup_repo(true) # dirty
+cpath = _setup_repo(false) # clean
+
+@test isdirty(dpath)
+@test endswith(gitdescribe(dpath), "_dirty")
+@test !isdirty(cpath)
+@test !endswith(gitdescribe(cpath), "_dirty")
 
 # tag!
+function _test_tag!(d, path, haspatch, DRWATSON_STOREPATCH)
+    d = copy(d)
+    withenv("DRWATSON_STOREPATCH" => DRWATSON_STOREPATCH) do
+        d = tag!(d, gitpath=path)
+        commitname = keytype(d)(:gitcommit)
+        @test haskey(d, commitname)
+        @test d[commitname] isa String
+        if haspatch
+            patchname = keytype(d)(:gitpatch)
+            @test haskey(d, patchname)
+            @test d[patchname] isa String
+            @test d[patchname] != ""
+        end
+    end
+end
+
 d1 = Dict(:x => 3, :y => 4)
 d2 = Dict("x" => 3, "y" => 4)
-for d in (d1, d2)
-    d = tag!(d, gitpath=@__DIR__)
-
-    @test haskey(d, keytype(d)(:gitcommit))
-    @test d[keytype(d)(:gitcommit)] |> typeof <: String
+@testset "tag! ($(keytype(d)))" for d in (d1, d2)
+    @testset "no patch ($(dirty ? "dirty" : "clean"))" for dirty in (true, false)
+        path = dirty ? dpath : cpath
+        _test_tag!(d, path, false, nothing) # variable unset
+        _test_tag!(d, path, false, "") # variable set but empty
+        _test_tag!(d, path, false, "false") # variable parses as false
+        _test_tag!(d, path, false, "0") # variable parses as false
+        _test_tag!(d, path, false, "rubbish") # variable not a Bool
+    end
+    @testset "patch" begin
+        _test_tag!(d, dpath, true, "true") # variable parses as true
+        _test_tag!(d, dpath, true, "1") # variable parses as true
+    end
 end
 
+# Ensure that above tests operated out-of-place.
+@test d1 == Dict(:x => 3, :y => 4)
+@test d2 == Dict("x" => 3, "y" => 4)
+
 # Test assertion error when the data has a incompatible key type
 @test_throws AssertionError("We only know how to tag dictionaries that have keys that are strings or symbols") tag!(Dict{Int64,Any}(1 => 2))
 @test_throws AssertionError("We only know how to tag dictionaries that have keys that are strings or symbols") DrWatson.scripttag!(Dict{Int64,Any}(1 => 2), "foo")
 
 
 # @tag!
-for d in (d1, d2)
+@testset "@tag! ($(keytype(d)))" for d in (d1, d2)
     d = @tag!(d, gitpath=@__DIR__)
     @test haskey(d, keytype(d)(:gitcommit))
     @test d[keytype(d)(:gitcommit)] |> typeof <: String
@@ -318,7 +359,7 @@ rm(tmpdir, force = true, recursive = true)
     n = @ntuple x y
     @test isa(ntuple2dict(n),Dict)
     @test isa(ntuple2dict(OrderedDict,n),OrderedDict)
-    
+
     #test checktagtype!
     @test isa(DrWatson.checktagtype!(d3),Dict)
     @test isa(DrWatson.checktagtype!(d11),OrderedDict)