FactCheck.jl

FactCheck.jl copied to clipboard

Given the preponderance of statistical and scientific Julia code, I think this pair of assertion helpers are very useful. I use them for unit-testing statistical expectations of stochastic functions. For example,

using Distributions

lower, upper = quantile(Binomial(100, 0.5 * 0.5), [0.01, 0.99])
@fact complicated_function(...) --> within_inclusive(lower, upper)

Seeing the expected lower and upper bound in the failing test output makes my life easier.

This seems overkill. Why not complicated_function() in 1:<range> --> true?

I think that has two problems.

1) The error messages are noisy.

@fact 9.0 in 10.0:12.0 --> true

Failure :: (line:507) :: fact was false
    Expression: $(Expr(:in, 9.0, :(10.0:12.0))) --> true
      Expected: true
      Occurred: false

versus

@fact 9.0 --> within(10.0, 12.0)

Failure :: (line:507) :: fact was false
   Expression: 9.0 --> within(10.0,12.0)
     Expected: 9.0 --> within(10.0)

The printed expectation violation is wrong, but I wanted to do as little surgery to FactCheck's special cases as possible. And, the Expression print is very clear.

2) It's doubly confusing with inclusive bounds.

@fact 1 in (1-1):(2+1) --> true

I think I'd tend to agree with you in that special cases don't belong in FactCheck. But, I think this style of test comes up a lot.

Future Work...

I think someting like an AssertionHelper abstract type is a good idea, in general. Derived-types would implement methods for equality checking and better error messages. This allows for well-designed semantics without the need for FactCheck commits.

I would support within(::Range) helper method. Range is already inclusive so within_inclusive is redundant.

within could be a generalization of in collection containment helper function which could give you the better output that you want above.

Hrm. I made a mistake with the example. The non-inclusive within is not captured by a range. But, that's an edge case, even for distribution testing. And, I think it could be captured by not(within(l,u)) but I haven't looked at how not works yet. Either way, it's less of an issue.

I think you are right -- a within(::Range) method is cleaner. I'll try it out tonight, leaving this unclosed until then.

Reimplemented, as per discussion with @jakebolewski. It does feel cleaner than my original implementation. The failure presentation is mostly better. It shows lower:step:upper for FloatRange. But, that doesn't run afoul of the principle of least astonishment. Non-inclusive is a simple not(within(x:y)).

Actually, no. That's not right either. It doesn't work for FloatRanges. The step field is honored by in. This won't work for the most common use case of continuous distribution confidence intervals. Let me know what alternative implementation -- other than my original one of within(lower, upper) -- will pass muster for a merge.