Test function TestGetWithin randomly fails

[obdeijn]

Test function TestGetWithin randomly fails

Summary

When I run the test suite using "make test" I sometimes run into the following failing test:

=== RUN   TestGetWithin
    TestGetWithin: utils_test.go:460: 
        	Error Trace:	utils_test.go:460
        	Error:      	Not equal: 
        	            	expected: 359000000000
        	            	actual  : 360000000000
        	Test:       	TestGetWithin

Other times it succeeds:

=== RUN   TestGetWithin
--- PASS: TestGetWithin (0.00s)

Environment

• OS: Mac OS
• Kernel: Darwin AEO.local 19.3.0 Darwin Kernel Version 19.3.0: Thu Jan 9 20:58:23 PST 2020; root:xnu-6153.81.5~1/RELEASE_X86_64 x86_64
• go version 1.14.3
• Server: none
• Louketo: tag 10.0.0

Expected Results

I expect the test to always succeed.

Actual Results

The test sometimes succeeds, sometimes fails.

Steps to reproduce

Run the tests a few times (see description above).

Additional Information

Looking a the test, it seems to me like this is a timing issue, which I think is undesirable for a unit test.

[abstractj]

@obdeijn I could not reproduce the steps you mentioned. Here is my environment:

Environment

• Linux version 5.6.14-300/Fedora 32
• go version go1.14.3 linux/amd6
• Louketo tag: 10.0.0 and also tried with master

Steps

• Ran make test several times with no errors
• Ran go test -run TestGetWithin and also got no errors

@ASzc whenever you get the chance, could you please double check?

[ASzc]

Looking at the code of the test and getWithin, I think it's possible for the reported failure to occur if the test is executed at the wrong fraction of a second, so that the second rolls over between the test initializing and the function under test being called. This could be due to the usage of time.Now() in both places.

I'm not sure what the solution could be yet. For now I'll write something to rerun the TestGetWithin test as rapidly as possible, to see if I can get the reproduce the reported result.

[ASzc]

I haven't been able to reproduce, leaving the test going on loop for a while. I think it's still an issue, but only with the test. The getWithin function doesn't seem to really be used anywhere that needs super precision, and its callers also aren't also calling time.Now().

I suggest we just make the equality assertion in the test fuzzy, e.g. equals within 1000000000 units. That way if the freak accident of rollover happens, then the test doesn't fail.

I can make the PR if we agree on this solution.

[abstractj]

@ASzc sure, go for it.

[abstractj]

Marking it as enhancement as we were not able to reproduce any bug.