RFR:JDK-8163330:HijrahDate aligned day of week incorrect

[Anubhav Meena]

Thanks Roger, appreciate that.

> On Oct 13, 2016, at 3:09 AM, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
> 
> Hi,
> 
> Looks ok.
> 
> Typically, there is a space after the '//' comment characters, it makes it easier to read.
> Also, in this case, I don't think the comments help.  "Monday" isn't important to the test 
> and the 'Any Other day' is still 1, not something else.
> 
> I would remove those comments.
> Otherwise fine, no need for another webrev.
> 
> Thanks, Roger
> 
> 
> On 10/12/2016 2:16 PM, Anubhav Meena 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/ <http://cr.openjdk.java.net/%7Erchamyal/anmeena/8163330/webrev.04/>
>> 
>> Thanks,
>> Anubhav
>> 
>>> On Oct 12, 2016, at 8:45 PM, Roger Riggs <Roger.Riggs at Oracle.com <mailto:Roger.Riggs at 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.meena at oracle.com <mailto:anubhav.meena at oracle.com>> wrote:
>>>>> 
>>>>> Gentle reminder.
>>>>> 
>>>>>> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.meena at oracle.com <mailto:anubhav.meena at 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
>>>>> 
>>>> 
>>> 
>> 
>