Guava's tests check the explicit definition of square root (mentioned by Joe above) on 2^n +/- 1 for all n up to Double.MAX_EXPONENT + 1, because why not?
On Wed, Dec 9, 2015 at 6:12 PM Joseph D. Darcy <joe.da...@oracle.com> wrote: > Hi Brian, > > New version looks good. > > One more case to try: start with a BigInteger that would overflow to > Double.POSITIVE_INFINITY when the doubleValue method was called. If this > case doesn't take too long to run, it would be a fine additional case to > add to the test. 2^1024 should be fine input value. More precisely, > > (new > BigDecimal(Double.MAX_VALUE)).toBigInteger().add(BigInteger.ONE); > > should do the trick. If the code passes with this value, you're okay to > push. Well, while you're at it, might as well verify > > (new BigDecimal(Double.MAX_VALUE)).toBigInteger() > > behaves well too ;-) > > Thanks, > > -Joe > > On 12/9/2015 5:41 PM, Brian Burkhalter wrote: > > Hi Joe, > > > > On Dec 1, 2015, at 7:25 PM, Joseph D. Darcy <joe.da...@oracle.com > > <mailto:joe.da...@oracle.com>> wrote: > > > >> Current version looks okay. One more request, before pushing please > >> add explicit test cases for the for the largest number having 63 bits > >> and the smallest number having 64 bits. No need for another round of > >> webrevs for that. > > > > Well there is after all a need for another round of review: > > > > http://cr.openjdk.java.net/~bpb/8032027/webrev.04/ > > <http://cr.openjdk.java.net/%7Ebpb/8032027/webrev.04/> > > > > That was a good call to add the above tests: one of them failed. This > > was found to be due to a floor() where there should have been a ceil(). > > > > Summary: > > > > MutableBigInteger: at line 1920 change Math.floor(.) to Math.ceil(.). > > BigIntegerTest: at lines 331-340 add testing of 2^N and 2^N - 1, 0 < N > > < 1024 > > > > Thanks, > > > > Brian > >
