Converting JUnit tests into TestNG controlled environment

• Key: AVRO-26
• Type: Test
• Status: Closed
• Resolution: Fixed
• Priority: Major
• Assignee: Konstantin Boudnik
• Reporter: Konstantin Boudnik
• Component/s: java, python
• Fix Version/s: 1.0.0

Description

TestNG is a powerful test harness, which provides a lot of useful features like parallel test execution, test parametrization, and such.
Avro uses JUnit environment instead which is more rigid and less efficient.

I'd suggest to convert Avro's Junit based test infrastructure into TestNG controller environment, which will allow to achieve higher level of test execution control in the long run

Comments

[Konstantin Boudnik]

This patch includes the following modifications:

• JUnit tests conversion to be naively executed by TestNG. This involves new annotations and inter tests dependencies introduction
• some of test files for schemata were renamed and affected Java and Python code has been refactored accordingly
• build.xml file is modified to accommodate TestNG switch
• an extra target 'reports' has been added to facilitate conversion of the reports produced by testng into junit format
• import lists are optimized for all test classes for they included too much of unused imports which made tests refactoring difficult
• new library is added to lib/testng-5.9-jdk15.jar

Tests invocation remain the same: it could be executed either through 'test' for all languages or through 'test-java' for java. Interop test interfaces are remained as before too.

[Konstantin Boudnik]

Patch has been attached for the review and comments or approval.
Please let me know if someone has any concerns or suggestions.

[Konstantin Boudnik]

This patch uses @BeforeMethod/@AfterMethod annotations to replace hardcoded inter-tests dependencies.

With this technique the overall maintenance would be significantly lower.

Consider this patch candidate instead of earlier submitted AVRO-26.patch

[Doug Cutting]

Overall this looks good. A few comments:

• 'generated' is not a good name for this. let's instead name it something like "hello", "simple" or "testProtocol".
• if testng is in lib/, then we don't need testng.classpath but can just use the normal classpath. we could instead put it in src/test/lib or somesuch. if we do that, build.xml should not contain the full version-laden file name, but rather a version-free directory, so that we can change testng versions without editing build.xml.
• we should continue to support something like 'ant test-java -Dtestcase=TestFoo', to permit folks to run just a single test.
• i would not update the ant task descriptions to say "with TestNG"
• do we need the taskdef in more than one place in build.xml?
• the "reports" task should probably be named "test-java-reports" or "test-reports".
• i must be the only person in the world who likes to use asterisks in imports...

[Konstantin Boudnik]

Thanks for the comments, Doug.

Couple of replies:

• TestProtocol kinda imply that other TestProtocol* classes are in a
  sort of relation with this one, which I believe isn't true. So I went
  with Simple
• agree, this is an artifact of my own environment where testng was
  placed separately. Fixed. It sounds like a good idea to move test
  related libs to src/test/lib. And the version part has to be stripped.
  However, if Avro will move toward Ivy it will be changed anyways, right?
  Also, it seems that junit-4.5.jar sits in the lib directory as well
• done, sorry I've missed this piece of original functionality
• looks like testdefs are needed in every target in case someone will
  have to run test-interop-data-java. If testng isn't define within that
  target then the testing will fail
• test-reports it is
• do you want me to revert the tests' import lists?

Also, I'd suggest to rename compile-java-test to compile-test-java to be
in line with other test related names such as test-java and so on. I did
it as a part of new version of the submitted patch.

Cos

[Konstantin Boudnik]

• generated.js is renamed to simple.js to avoid confusions
• testng.classpath is removed from the build file for the jar file is the part of the project's java.classpath
• an ability to specify a single testcase is added. Simple set -Dtestcase=<shortname_of_test> and will be executed
• reports renamed to test-reports
• compile-java-test is renamed to compile-test-java for the similarity with other test related targets

[Konstantin Boudnik]

dumpCommand="true" is no longer needed in the production version of the build.xml

[Doug Cutting]

A few comments on the latest patch:

• we should remove lib/junit*.jar
• TestValueReader has not yet been converted to testng
• please attach a shell script, AVRO-26.sh, that can be run before the patch is applied to perform any 'svn mv' and 'svn rm' operations required
• please attach the testng jar too.
• in build.xml, the taskdef can be moved outside the target to top-level and then need not be repeated. (we should eventually similarly fix the py-* taskdefs.)

[Konstantin Boudnik]

All comment's from Doug last review are taken into account. Namely:

• shell script is created to deal with file/folder modifications (sorry, I keep forgetting that I have no commit rights and someone else has to do this for me)
• forgotten test is converted into testng format
• a number of taskdef's is reduced through pulling up one of them
• jar file is attached

[Doug Cutting]

The patch looks good now.

I miss JUnit's plain text output that included any print statements. Perhaps we could add a listener that prints each test run along with its success or failure?

[Konstantin Boudnik]

Thanks for the reviews, Doug.

The other day we have a short discussion about this with Nigel and I've
implemented something similar to what you're asking for in another project I'm
working on right now. I'll try to drop that piece of reporting listener to
Avro's test base by end of the week and send out another patch.

Cos

[Konstantin Boudnik]

This patch introduces a solution for JUnit like plain test reporting output. In order to use it one needs to modify existing listeners line of build.xml to
listeners="org.apache.avro.MyOutputInterceptor, org.apache.avro.SuiteInterceptor"

As the result, testng runs will produce something like this (although, properly aligned)

[testng] org.apache.avro.TestSchema.testString Pass ( 19 ms)
[testng] org.apache.avro.TestSchema.testLong Pass ( 4 ms)
[testng] org.apache.avro.TestSchema.testUnion Pass ( 10 ms)
[testng] org.apache.avro.TestSchema.testArray Pass ( 11 ms)
[testng] org.apache.avro.TestSchema.testNull Pass ( 1 ms)
[testng] org.apache.avro.TestSchema.testLisp Pass ( 30 ms)
[testng] org.apache.avro.TestSchema.testRecursive Pass ( 75 ms)
[testng] org.apache.avro.TestSchema.testRecord Pass ( 5 ms)
[testng] org.apache.avro.TestSchema.testFloat Pass ( 2 ms)
[testng] org.apache.avro.TestSchema.testInt Pass ( 1 ms)
[testng] org.apache.avro.TestSchema.testDouble Pass ( 3 ms)
[testng] org.apache.avro.TestSchema.testMap Pass ( 16 ms)
[testng] org.apache.avro.TestSchema.testBytes Pass ( 3 ms)
[testng] org.apache.avro.TestSchema.testBoolean Pass ( 2 ms)
[testng] org.apache.avro.io.TestValueReader.testEOFHandling Pass ( 1 ms)
[testng] AvroTestNG
[testng] Total tests run: passed 34; failed 0; skipped 0 in 1 s.

[Doug Cutting]

This sounds nice.

• Can you please merge your patches together, so that one need only apply a single patch to test it?
• Let's call these TestInvocationReporter and TestOutputInterceptor, and put them in package org.apache.avro.test.
• Should we print to System.out or System.err? You've done one in some places and the other in others. We should probably be consistent.

[Konstantin Boudnik]

Will merge these in a second: I felt like sending them separately in order to
present them separately. And renaming them totally makes sense.

Wrt placement of the classes: shall we pull them off avro at all, so other
projects which will be coming toward testng harness will be able to reuse
them? Say, org.apache.hadoop.common.test ?

I'd vote for System.out - these are normal reporting output. System.err is
left there as a result of some previous experimenting. Will be gone in the
coming patch.

Thanks,
Cos

[Doug Cutting]

> org.apache.hadoop.common.test ?

Let's wait until we move them to do that.

[Konstantin Boudnik]

Output interceptors are moved to org.apache.avro.test package
Modifications of JSON schema are merged into the test sources

[Doug Cutting]

I just committed this. Thanks, Konstantin.

[Konstantin Boudnik]

Just to give a heads-up on the status of the progress of JUnit tests conversion into TestNG in the Hadoop.

It seems that Hadoop and ZooKeeper are backing off from the whole TestNG thing, because the transition's overhead is too big to bear. Please contact me directly if you

The general approach for the rest of Hadoop is to keep going with JUnit framework and solve our tagging needs with JUnit's test suite mechanism.

Considering the future merging of Avro to Hadoop and an obvious requirement of having a single test framework, I'd suggest to think of 'reverse' conversion of TestNG tests back to JUnit. I think a mere exclusion of the current patch won't work.

[Doug Cutting]

Konstantin, can you please file a new issue to switch Avro back to JUnit? Thanks!