Hi Olivier, On Sep 22, 2014, at 7:56 AM, olivier.lagn...@oracle.com wrote:
>> and 2) use braces around all the statements contained in if/else blocks (see >> below). Comment #2 is nit-picky. > I tried to keep the same flavour of writing as in HALF_DOWN and HALF_EVEN > cases, i.e. don't use brace for > terminal/leaf return true/false statements. This is not the standard however, > at least in this file. > Will use braces in all case (i.e. the 3 of HALF_UP, HALF_DOWN and HALF_EVEN). I did not look at the other case. If your formatting matches the rest of the file then I think it is OK to leave it as-is. >> Lastly and this is not really part of your changeset, but I found it curious >> that the test prints the details of all cases that succeed as opposed to >> those that fail. I would think that either all results or the failures alone >> ought to be printed instead of successes only. See for example the partial >> diff below the DigitList diff. > Since these are most often corner and tricky test cases I think it > interesting to have the details of each result, > including infos of both why returned result is correct or wrong. > That can help the reader to understand all these tricky cases. > The bad side of it being that it prints a lot of text, with failure cases > (hoepfully few) lost in the middle of it, > thus making failures possibly not immediate to detect. > > Here is an example of what is printed in case of failure: > " > ======================================== > ***Error formatting double value from string : 0.6868d > NumberFormat pattern is : #,##0.### > Maximum number of fractional digits : 3 > Fractional rounding digit : 4 > Position of value relative to tie : above > Rounding Mode : HALF_UP > BigDecimal output : 0.68679999999999996607158436745521612465381622314453125 > FloatingDecimal output : 0.6868 > Error. Formatted result different from expected. > Expected output is : "0.687" > Formated output is : "0.686" > ======================================== I missed that output: I was looking for the word “failure.” > There is also a reminder of the number of errors at the end of the report: > " > ==> 4 tests failed > > Test failed with 4 formating error(s). > " > > May be providing a reminder (value + rounding-mode + rounding position) > of the failure cases at the end of the would be better ? > Like: > " > Test failed with 4 formating error(s) for following cases : > - 0.3126d, HALF_UP rounding, 3 fractional digits > - 0.6868d, HALF_UP rounding, 3 fractional digits > - 1.798876d, HALF_UP rounding, 5 fractional digits > - 1.796889d, HALF_UP rounding, 5 fractional digits > " > > Would doing so be ok ? If the test is already printing out the information you showed above (“Error formatting …”) then I think it is enough but the verbiage should perhaps match the reminder, e.g., “Failure: Error formatting double …” Thanks, Brian
