Harmony
  1. Harmony
  2. HARMONY-6181

[classlib][text] SimpleDateFormatTest.test_set2DigitYearStartLjava_util_Date would fail

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0M9
    • Fix Version/s: 5.0M10
    • Component/s: Classlib
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      Currently the testcase test_set2DigitYearStartLjava_util_Date of SimpleDateFormatTest would fail as below[1]. After investigation, I noticed that this failure was caused by the different interpretation for pattern "y" between ICU and RI. In ICU spec, it explicitly mentioned "ICU interprets a single 'y' differently than Java." As ICU's behavior doesn't comply with the Java spec, it results in the Harmony defect in fact. Luckily, I've got a fix by doing a trick to solve this issue.

      [1] Failure trace:
      junit.framework.AssertionFailedError: Incorrect year 2000 expected:<2000> but was:<2>
      at junit.framework.Assert.fail(Assert.java:47)
      at junit.framework.Assert.failNotEquals(Assert.java:277)
      at junit.framework.Assert.assertEquals(Assert.java:64)
      at junit.framework.Assert.assertEquals(Assert.java:195)
      at org.apache.harmony.text.tests.java.text.SimpleDateFormatTest.test_set2DigitYearStartLjava_util_Date(SimpleDateFormatTest.java:797)
      at java.lang.reflect.AccessibleObject.invokeV(AccessibleObject.java:25)
      at java.lang.reflect.Method.invoke(Method.java:258)
      at junit.framework.TestCase.runTest(TestCase.java:164)
      at junit.framework.TestCase.runBare(TestCase.java:130)
      at junit.framework.TestResult$1.protect(TestResult.java:110)
      at junit.framework.TestResult.runProtected(TestResult.java:128)
      at junit.framework.TestResult.run(TestResult.java:113)
      at junit.framework.TestCase.run(TestCase.java:120)
      at junit.framework.TestSuite.runTest(TestSuite.java:228)
      at junit.framework.TestSuite.run(TestSuite.java:223)
      at org.junit.internal.runners.OldTestClassRunner.run(OldTestClassRunner.java:35)
      at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:38)
      at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
      at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:460)
      at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:673)
      at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:386)
      at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:196)

      1. HARMONY-6181_V2.diff
        2 kB
        Jim Yu
      2. HARMONY-6181.diff
        0.8 kB
        Jim Yu

        Activity

        Hide
        Tim Ellison added a comment -

        Thanks Jim.

        Patch applied to TEXT module at repo revision r768920.

        Please check it was applied as you expected. I assume the test case will be removed from the exclude list in time.

        Show
        Tim Ellison added a comment - Thanks Jim. Patch applied to TEXT module at repo revision r768920. Please check it was applied as you expected. I assume the test case will be removed from the exclude list in time.
        Hide
        Jim Yu added a comment -

        Thanks, Tim. But I found the earlier patch only solved the case for pattern "y". However, there may be complex pattern which contains 'y' as a sub pattern. For that case, we need a general solution to replace the 'y' in a complex pattern with 'yy'. I've attached a new patch HARMONY-6181_V2.diff to provide a general solution.

        Show
        Jim Yu added a comment - Thanks, Tim. But I found the earlier patch only solved the case for pattern "y". However, there may be complex pattern which contains 'y' as a sub pattern. For that case, we need a general solution to replace the 'y' in a complex pattern with 'yy'. I've attached a new patch HARMONY-6181 _V2.diff to provide a general solution.
        Hide
        Jim Yu added a comment -

        Reopen this JIRA as an improved patch is available.

        Show
        Jim Yu added a comment - Reopen this JIRA as an improved patch is available.
        Hide
        Jim Yu added a comment -

        After solving this failure, there are still four testcase failures for SimpleDateFormatTest. I will continue to investigate for the others.

        Show
        Jim Yu added a comment - After solving this failure, there are still four testcase failures for SimpleDateFormatTest. I will continue to investigate for the others.
        Hide
        Tim Ellison added a comment -

        Jim,

        I applied a slightly modified version of your v2 patch to the TEXT module at repo revision r772140.

        The changes were superficial.

        Please verify this now fixes the issue.

        Show
        Tim Ellison added a comment - Jim, I applied a slightly modified version of your v2 patch to the TEXT module at repo revision r772140. The changes were superficial. Please verify this now fixes the issue.
        Hide
        Jim Yu added a comment -

        Thanks, Tim!

        I just found a minor problem here. I think it should be changed as following.

        Index: modules/text/src/main/java/java/text/SimpleDateFormat.java
        ===================================================================
        — modules/text/src/main/java/java/text/SimpleDateFormat.java (revision 772171)
        +++ modules/text/src/main/java/java/text/SimpleDateFormat.java (working copy)
        @@ -266,7 +266,7 @@

        • spec.
          */
          String templateForICU = patternForICU(template);
        • icuFormat.applyPattern(template);
          + icuFormat.applyPattern(templateForICU);
          pattern = template;
          }
        Show
        Jim Yu added a comment - Thanks, Tim! I just found a minor problem here. I think it should be changed as following. Index: modules/text/src/main/java/java/text/SimpleDateFormat.java =================================================================== — modules/text/src/main/java/java/text/SimpleDateFormat.java (revision 772171) +++ modules/text/src/main/java/java/text/SimpleDateFormat.java (working copy) @@ -266,7 +266,7 @@ spec. */ String templateForICU = patternForICU(template); icuFormat.applyPattern(template); + icuFormat.applyPattern(templateForICU); pattern = template; }
        Hide
        Tim Ellison added a comment -

        Ouch! Good catch!

        Fixed in r772196.

        Show
        Tim Ellison added a comment - Ouch! Good catch! Fixed in r772196.
        Hide
        Jim Yu added a comment -

        Verified at r772209. Thanks, Tim.

        Show
        Jim Yu added a comment - Verified at r772209. Thanks, Tim.

          People

          • Assignee:
            Tim Ellison
            Reporter:
            Jim Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Time Tracking

              Estimated:
              Original Estimate - 24h
              24h
              Remaining:
              Remaining Estimate - 24h
              24h
              Logged:
              Time Spent - Not Specified
              Not Specified

                Development