Uploaded image for project: 'Harmony'
  1. Harmony
  2. HARMONY-6181

[classlib][text] SimpleDateFormatTest.test_set2DigitYearStartLjava_util_Date would fail

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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
        junjie0122 Jim Yu added a comment -

        Verified at r772209. Thanks, Tim.

        Show
        junjie0122 Jim Yu added a comment - Verified at r772209. Thanks, Tim.
        Hide
        tellison Tim Ellison added a comment -

        Ouch! Good catch!

        Fixed in r772196.

        Show
        tellison Tim Ellison added a comment - Ouch! Good catch! Fixed in r772196.
        Hide
        junjie0122 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
        junjie0122 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
        tellison 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
        tellison 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
        junjie0122 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
        junjie0122 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
        junjie0122 Jim Yu added a comment -

        Reopen this JIRA as an improved patch is available.

        Show
        junjie0122 Jim Yu added a comment - Reopen this JIRA as an improved patch is available.
        Hide
        junjie0122 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
        junjie0122 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
        tellison 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
        tellison 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.

          People

          • Assignee:
            tellison Tim Ellison
            Reporter:
            junjie0122 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