MaxGekk commented on PR #58850: URL: https://github.com/apache/spark/pull/58850#issuecomment-5703760292
Both points are right, and both are fixed in 5757614. The interval fixture was a real hole rather than a cosmetic one: the step was 7_001_000 microseconds, a whole number of milliseconds, so a lane that came back rounded to the millisecond would have compared equal and the case would have passed while its name claimed otherwise. The step is 7_001_001 microseconds now, and 999 of the 1000 values are not whole milliseconds. On "every precision": the mismatch was wider than four against the title, since `TimeType` admits 0 to 9 and the loop ran four of the ten. It now runs `TimeType.MIN_PRECISION to TimeType.MAX_PRECISION`, so the claim is the type's own rather than a list that can drift from it, and it stays true if the range ever widens. Thirty cases, about 5 seconds. Both fixtures now assert the property they are supposed to have instead of trusting the arithmetic that produces it - every generated TIME value is a whole number of its precision's unit and some value uses the last digit that precision admits; every generated interval is a whole number of microseconds while more than half are not whole milliseconds - and they check the generated values before the null pattern hides any of them, so the all-null case exercises the generator like the others. Whole suite green (79 tests), `dev/scalastyle` passes. Thank you for both readings; the millisecond one in particular would have left the test looking stronger than it was. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
