elm-test icon indicating copy to clipboard operation
elm-test copied to clipboard

Remove AbsoluteOrRelative floating point tolerance in favor of Expect.any?

Open mgold opened this issue 7 years ago • 5 comments

Currently we define AbsoluteOrRelative : Float -> Float -> FloatingPointTolerance. It would be a major change to remove the constructor, but I wonder if with Expect.any (#228) if it's still necessary.

So code that currently looks like this

floatingPointValue |> Expect.within (AbsoluteOrRelative 0.5 1.2) target
-- would now look like this
floatingPointValue |> Expect.any
  [ Expect.within (Absolute 0.5) target
  , Expect.within (Relative 1.2) target
  ]

It's definitely more code though, so I'm not completely sure. I wanted to raise the idea though.

mgold avatar Feb 13 '18 06:02 mgold

Good question. I think it would make people way more likely to use absolute xor relative tolerance, even if they should really be using both.

drathier avatar Feb 13 '18 06:02 drathier

How do you mean? Currently AbsoluteOrRelative uses logical OR (or at least that's what's documented...). With Expect.all and Expect.any you could get AND or OR. XOR is the one I don't know how you'd get.

Another possibility is Expect.withinRelative and Expect.withinAbsolute. I think that would get clunky with the NOT versions, though.

mgold avatar Feb 16 '18 06:02 mgold

Sorry, I was unclear. I ment that people will probably not write both an absolute and a relative expectation for a test and join them up with Expect.any, but rather just use one of them. I'd also expect many people to just use absolute for all tests, completely ignoring relative, or maybe vice versa.

I could also see how people would try writing fuzz tests with just an absolute tolerance, get a failing fuzz test, try some more, get annoyed that they cannot write a fuzz test that properly checks their code (i.e. assuming the code is correct and that the fuzz test is not), and then write a unit test instead of a fuzz test. I think it's much less obvious that you need to use Expect.any than that you need to use the correct constructor for the adt, even if the adt still isn't perfect. I very rarely even think of using Expect.any and Expect.all right now. I need them so seldom when I write tests that I pretty much forget that they exist.

drathier avatar Feb 16 '18 06:02 drathier

Ah, that's a little clearer. But we'd still have two tags in the ADT, Absolute Float and Relative Float. Are you saying that someone with a failing fuzz test is more likely to see AbsoluteOrRelative Float Float and say "Ah ha!" than if it's not there? I mean... maybe?

mgold avatar Feb 19 '18 01:02 mgold

Yeah. I think less code makes people more likely to use it, and that people are more likely to look at the constructors of an ADT than to go search for a different function. It's hard to tell for sure.

drathier avatar Feb 19 '18 07:02 drathier