Skip to content
This repository has been archived by the owner on Oct 9, 2019. It is now read-only.

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

Open
mgold opened this issue Feb 13, 2018 · 5 comments
Open

Comments

@mgold
Copy link
Member

mgold commented Feb 13, 2018

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.

@drathier
Copy link
Collaborator

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.

@mgold
Copy link
Member Author

mgold commented Feb 16, 2018

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.

@drathier
Copy link
Collaborator

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.

@mgold
Copy link
Member Author

mgold commented Feb 19, 2018

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?

@drathier
Copy link
Collaborator

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants