+1 Stephen
On 12 October 2016 at 19:16, Anubhav Meena <anubhav.me...@oracle.com> wrote: > 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/ > > 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> wrote: > > Gentle reminder. > > On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.me...@oracle.com> wrote: > > Hi all, > > Please review. > > Bug Id : 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/ > > -- > Thanks and Regards, > > Anubhav > > > > >
