Testing float-valued functions

[Eric S. Raymond]

Ian Bruene via devel <devel at ntpsec.org>:
> I changed ntp_to_posix back to what it should be, and fixed what tests I can
> fix immediately.

Good.

> >A question for you to ponder: should you replace all assertEqual calls
> >or only those on which we have seen failures?  If the latter, what
> >else do you need to do?
> >
> >Be prepared to discuss the tradeoffs and justify your answer.
> 
> For ntp_to_posix and posixize yes, all of the float tests should be changed.
> Without changing them J. Random System might throw a fit at any time.

That is one correct answer.

But: not quite any time. In practice, you have two windows of vulnerability:

(1) When you add a new platform to supported ports.

(2) Compiler upgrades that change how FP code is generated.  And, actually,
we might not see one of those again, now that on-chip FP hardware conforming
to IEEE754 is ubiquitous.

Thus, the *other* correct answer would be to change the tests only as
needed, but document in the test code that this is what you are doing
and why.  Then count on the tests to break constructively on new
platforms.

Neither answer is wrong, they just have different tradeoffs.  

> For ntp_to_posix it is more complicated; it takes a float, so representation
> issues will arise. However it returns an integer, so a simple
> assertAlmostEqual() won't cut it. This will probably require some custom
> test code.

This is why the minimal-change alternative is worth considering at this
point in our release cycle.  It means you don't have to do that research
project.

> Today I will do a run through the tests to find other float-returning
> testees that need adjustment.

And, of course, you'll stay aware of the issue in the future. So this
is all good.

> A related issue is that I am unsure of the value of the Timeless Void tests.
> Those are testing values outside of what NTP represents, and in theory are
> the same kind of unnecessary test as force feeding a list to a function that
> expects a bool. On the other hand one of those "unnecessary" tests was the
> one that caught the bug.

True, but keep firmly in mind that this was a bug in *tests*, not a bug in
code.  You may indeed have ventured into over-testing here - it's rare, but it
does happen.

My advice is to keep the out-of-domain tests, but mark them as low-value
and disposable if passing them would incur additional complexity costs.

But exercise your judgment.  You own the Python code, so I'm only
advising you rather than issuing a diktat.  Part of your training is
making these decisions under conditions where you're allowed to call
things wrong and observe the consequences, provided doing so does not
significantly threaten our code quality.
-- 
		<a href="http://www.catb.org/~esr/">Eric S. Raymond</a>

My work is funded by the Internet Civil Engineering Institute: https://icei.org
Please visit their site and donate: the civilization you save might be your own.