On 10/06/2017 06:52 AM, Eric S. Raymond wrote:
In fact, all this test code is subtly wrong, and it is just blind luck nothing went sproing sooner. Any of those assertions could have gone toes-up at any time. The tests in posixize() are wrong, too. The problem here is that float representation is not exact, and the "same" float calculation on different platforms may yield slightly different results. Thus comparing floats to floats is flaky, and comparing ints to floats is flaky. This is true no matter what language you are using; it's an intrinsic property of the float data representation. [...] Ian, your assignment is to fix this and verify the fix. Please do this as immediately as possible.
I changed ntp_to_posix back to what it should be, and fixed what tests I can fix immediately.
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. 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.
Today I will do a run through the tests to find other float-returning testees that need adjustment.
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.
-- In the end; what separates a Man, from a Slave? Money? Power? No. A Man Chooses, a Slave Obeys. -- Andrew Ryan _______________________________________________ devel mailing list devel@ntpsec.org http://lists.ntpsec.org/mailman/listinfo/devel
