Avro
  1. Avro
  2. AVRO-81

Switch Avro's tests back to JUnit from current TestNG framework

    Details

    • Type: Test Test
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1.0
    • Component/s: java
    • Labels:
      None

      Description

      This issue has been opened in order to track possible future switch of testing framework for Avro. Currently it is based on TestNG, however because of a number of limitations this test framework is not likely to be used by any of Hadoop's subproject. Thus, Avro will have to be start using JUnit again.

      1. AVRO-81.patch
        45 kB
        Konstantin Boudnik
      2. AVRO-81.patch
        46 kB
        Konstantin Boudnik
      3. AVRO-81.sh
        0.3 kB
        Konstantin Boudnik
      4. AVRO-81.patch
        46 kB
        Thiruvalluvan M. G.
      5. AVRO-81.patch
        46 kB
        Konstantin Boudnik

        Issue Links

          Activity

          Konstantin Boudnik created issue -
          Konstantin Boudnik made changes -
          Field Original Value New Value
          Link This issue relates to AVRO-26 [ AVRO-26 ]
          Hide
          Konstantin Boudnik added a comment -

          This patch takes care about reverse conversion of the tests to JUnit framework.
          However, the following problem has been found and I haven't find the explanation yet:

          • in TestValidatingIO 81 out of 960 are failing with messages like this
            Testcase: testMain[8] took 0.004 sec
                    FAILED
            expected:<[B@17cd7867> but was:<[B@329bbe66>
            junit.framework.AssertionFailedError: expected:<[B@17cd7867> but was:<[B@329bbe66>
                    at org.apache.avro.io.TestValidatingIO.check(TestValidatingIO.java:377)
                    at org.apache.avro.io.TestValidatingIO.check(TestValidatingIO.java:308)
                    at org.apache.avro.io.TestValidatingIO.testOnce(TestValidatingIO.java:71)
                    at org.apache.avro.io.TestValidatingIO.testMain(TestValidatingIO.java:61)
            

          Also, an extra test class TestResolvingIO_resolving has been created to workaround the limitations of JUnit @Parameters solution, i.e. you can't have more than one data provider per test class.

          Show
          Konstantin Boudnik added a comment - This patch takes care about reverse conversion of the tests to JUnit framework. However, the following problem has been found and I haven't find the explanation yet: in TestValidatingIO 81 out of 960 are failing with messages like this Testcase: testMain[8] took 0.004 sec FAILED expected:<[B@17cd7867> but was:<[B@329bbe66> junit.framework.AssertionFailedError: expected:<[B@17cd7867> but was:<[B@329bbe66> at org.apache.avro.io.TestValidatingIO.check(TestValidatingIO.java:377) at org.apache.avro.io.TestValidatingIO.check(TestValidatingIO.java:308) at org.apache.avro.io.TestValidatingIO.testOnce(TestValidatingIO.java:71) at org.apache.avro.io.TestValidatingIO.testMain(TestValidatingIO.java:61) Also, an extra test class TestResolvingIO_resolving has been created to workaround the limitations of JUnit @Parameters solution, i.e. you can't have more than one data provider per test class.
          Konstantin Boudnik made changes -
          Attachment AVRO-81.patch [ 12414487 ]
          Hide
          Thiruvalluvan M. G. added a comment -

          This is the modified version of the patch by Konstantin. This fixes the problem reported by him. The trouble was testng can distinguish between arrays and scalars in assertEquals() and do the right thing. JUnit requires you to call assertArrayEquals() explicitly for arrays.

          Now those tests pass. Still I see three errors in TestInvocationReporter, TestOutputInterceptor and TestSuiteInterceptor.

          I've noticed the following (all of them subtle, not serious):

          • The classes from from both Junit4 and Junit3 are used. Junit3 package names start with "junit" and Junit4 package names with "org.junit". Though they seem to work together, it's better to stick to one version.
          • The order of arguments for assertEquals() is different from Testng and Junit. In junit, the first argument is the expected value and for testng it is the actual. Again, there is no harm if we use the wrong order because, after all, it checks for equality. But the error messages will be confusing.
          • The fields of parametrized test classes, it is possible to make the input fields final.

          I've fixed these for tests in avro.io package. Other packages may have these.

          I saw some opportunities, in Junit context, to refactor and reduce test code for avro.io package. I'll take that up once conversion to Junit is over.

          Show
          Thiruvalluvan M. G. added a comment - This is the modified version of the patch by Konstantin. This fixes the problem reported by him. The trouble was testng can distinguish between arrays and scalars in assertEquals() and do the right thing. JUnit requires you to call assertArrayEquals() explicitly for arrays. Now those tests pass. Still I see three errors in TestInvocationReporter, TestOutputInterceptor and TestSuiteInterceptor. I've noticed the following (all of them subtle, not serious): The classes from from both Junit4 and Junit3 are used. Junit3 package names start with "junit" and Junit4 package names with "org.junit". Though they seem to work together, it's better to stick to one version. The order of arguments for assertEquals() is different from Testng and Junit. In junit, the first argument is the expected value and for testng it is the actual. Again, there is no harm if we use the wrong order because, after all, it checks for equality. But the error messages will be confusing. The fields of parametrized test classes, it is possible to make the input fields final. I've fixed these for tests in avro.io package. Other packages may have these. I saw some opportunities, in Junit context, to refactor and reduce test code for avro.io package. I'll take that up once conversion to Junit is over.
          Thiruvalluvan M. G. made changes -
          Attachment AVRO-81.patch [ 12414491 ]
          Thiruvalluvan M. G. made changes -
          Attachment AVRO-81.patch [ 12414495 ]
          Thiruvalluvan M. G. made changes -
          Attachment AVRO-81.patch [ 12414491 ]
          Hide
          Konstantin Boudnik added a comment -

          Thanks for noticing this issue with array comparison - I really appreciate it: I've kinda stuck there.

          • all JUnit3 references are removed
          • all *Interceptors should be deleted (the script will take care about it)
          • finals for the class members make a lot of sense.

          Updated patch is attached: everything passes now.

          Show
          Konstantin Boudnik added a comment - Thanks for noticing this issue with array comparison - I really appreciate it: I've kinda stuck there. all JUnit3 references are removed all *Interceptors should be deleted (the script will take care about it) finals for the class members make a lot of sense. Updated patch is attached: everything passes now.
          Konstantin Boudnik made changes -
          Attachment AVRO-81.sh [ 12414641 ]
          Attachment AVRO-81.patch [ 12414642 ]
          Hide
          Doug Cutting added a comment -

          This is looking good. A few problems remain:

          • Build.xml has not been completely updated: the interop tests still use TestNG.
          • The patch contains a number of whitespace-only changes. Can you please remove these?
          • The patch comments out obsolete code in a few places. It should simply remove obsolete code. This is in build.xml and TestResolvingIO.
          Show
          Doug Cutting added a comment - This is looking good. A few problems remain: Build.xml has not been completely updated: the interop tests still use TestNG. The patch contains a number of whitespace-only changes. Can you please remove these? The patch comments out obsolete code in a few places. It should simply remove obsolete code. This is in build.xml and TestResolvingIO.
          Hide
          Konstantin Boudnik added a comment -
          • White space issues are fixed - my IDE keeps removing white spaces in the empty lines (which seems to be a sane thing to do, but creates these unnecessary modifications)
          • new macrodef has been introduced for junit task and all other test related targets are just setting needed arguments. This allow to eliminate a good chunk of duplicated code
          Show
          Konstantin Boudnik added a comment - White space issues are fixed - my IDE keeps removing white spaces in the empty lines (which seems to be a sane thing to do, but creates these unnecessary modifications) new macrodef has been introduced for junit task and all other test related targets are just setting needed arguments. This allow to eliminate a good chunk of duplicated code
          Konstantin Boudnik made changes -
          Attachment AVRO-81.patch [ 12414682 ]
          Konstantin Boudnik made changes -
          Attachment AVRO-81.patch [ 12414752 ]
          Konstantin Boudnik made changes -
          Attachment AVRO-81.patch [ 12414682 ]
          Hide
          Doug Cutting added a comment -

          I just committed this. Thanks, Konstantin!

          Show
          Doug Cutting added a comment - I just committed this. Thanks, Konstantin!
          Doug Cutting made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Fix Version/s 1.0.1 [ 12314106 ]
          Resolution Fixed [ 1 ]
          Doug Cutting made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Doug Cutting made changes -
          Component/s java [ 12312780 ]

            People

            • Assignee:
              Konstantin Boudnik
              Reporter:
              Konstantin Boudnik
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development