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

[Stephen Colebourne]

+1
Stephen

On 12 October 2016 at 19:16, Anubhav Meena <anubhav.meena at 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.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/
>
> 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> wrote:
>
> Gentle reminder.
>
> On Oct 7, 2016, at 2:12 PM, Anubhav Meena <anubhav.meena at 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
>
>
>
>
>