Hi Roger, Thanks for the suggestion, have made the suggested changes and shifted the tests to /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java.
Updated webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.04/ Thanks, Anubhav > On Oct 12, 2016, at 8:45 PM, Roger Riggs <roger.ri...@oracle.com> wrote: > > Hi Anubhav, > > In general, I agree with Stephen that the tests should be testing an > algorithm against facts. > Embedding an algorithm in the test increases the risk that the test will just > replicate the > implementation code and therefor not be much of a test. > > Though in this case, the specification of aligned day of week is of a > computation. > If the test were to independently compute the correct answer, it would be > valid as a 'tck' test. > > Since the Hijrah calendar is data driven, the tests should be in > /java/time/test/java/time/chrono/TestUmmAlQuraChronology.java. > Tests in java/time/tck/... should correspond directly to specified behaviors. > In this case, the algorithm is specified but the test is data dependent. > (Perhaps a gray area). > > Thanks, Roger > > > On 10/12/2016 11:03 AM, Anubhav Meena wrote: >> Hi Stephen, >> >> Have incorporated the changes you suggested. Updated webrev is available >> here >> http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.03/ >> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.03/> >> >> Please review and suggest if anymore changes are required. >> >> Thanks, >> Anubhav >> >>> On Oct 12, 2016, at 3:21 PM, Anubhav Meena <anubhav.me...@oracle.com >>> <mailto:anubhav.me...@oracle.com>> wrote: >>> >>> Gentle reminder. >>> >>>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com >>>> <mailto:anubhav.me...@oracle.com>> wrote: >>>> >>>> Hi all, >>>> >>>> Please review. >>>> Bug Id : https://bugs.openjdk.java.net/browse/JDK-8163330 >>>> <https://bugs.openjdk.java.net/browse/JDK-8163330> >>>> >>>> Issue: The HijrahDate class incorrectly calculates the >>>> aligned-day-of-week field. It based the calculation on the day-of-week, >>>> when it should be based on the day-of-month. >>>> >>>> Webrev: http://cr.openjdk.java.net/~rchamyal/anmeena/8163330/webrev.02/ >>>> <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.02/> >>>> -- >>>> Thanks and Regards, >>>> Anubhav >>> >> >
