invenia / Models.jl

An interface package that defines the methods and types for working with models.
MIT License
3 stars 0 forks source link

Remove return which breaks testsets #16

Closed sdl1 closed 4 years ago

sdl1 commented 4 years ago

Closes #15

Gives correct behaviour in the following situation:

julia> results = @testset "test" begin
              test_interface(FakeTemplate{PointEstimate, SingleOutput}())
              @test false
           end
test: Test Failed at REPL[3]:3
  Expression: false
Stacktrace:
 [1] top-level scope at REPL[3]:3
 [2] top-level scope at /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.4/Test/src/Test.jl:1113
 [3] top-level scope at REPL[3]:2
Test Summary:                             | Pass  Fail  Total
test                                      |   13     1     14
  Models API Interface Test: FakeTemplate |   13           13
ERROR: Some tests did not pass: 13 passed, 1 failed, 0 errored, 0 broken.

julia> results.anynonpass
ERROR: UndefVarError: results not defined
codecov[bot] commented 4 years ago

Codecov Report

Merging #16 into master will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #16   +/-   ##
=======================================
  Coverage   93.15%   93.15%           
=======================================
  Files           3        3           
  Lines          73       73           
=======================================
  Hits           68       68           
  Misses          5        5           
Impacted Files Coverage Δ
src/test_utils.jl 92.53% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5506930...5793dbf. Read the comment docs.

oxinabox commented 4 years ago

docstring for test_interface says:

Returns the predictions of theModel`.

However, I checked all packages that i know of that use Models.jl and none of them actually use this feature anymore.

I think should remove that docstring, and tag a breaking release.

Technically, you could rely on that feature right now, its just that your tests wouldn't work so great.

sdl1 commented 4 years ago

Cool, I removed the docstring and changed the version to be breaking

oxinabox commented 4 years ago

Cool. merge then register release. Registrering a release on github is done by following these instructions. We have the Registrator app installed already.