|
This looks great. Can you go ahead and list these tests into a Junit test suite class? I think that is the best way for now to capture them into a single suite that is run by the commit build.
Can you also coordinate with
Another option would be to specify the tests in a flat file (ten-minute-test or such): **/TestHDFSCLI.java **/TestUrlStreamHandler.java **/TestBlockReplacement.java **/TestDirectoryScanner.java **/TestDiskError.java etc... and then reference that file from within the ant target: <fileset dir="${test.src.dir}/hdfs" casesensitive="yes">
<patternset>
<includesfile name="ten-minute-test" />
</patternset>
</fileset>
This would make it easier to change the tests included, without having to modify any code. Might be less opaque. Also, regarding the project split (which was done after my performance numbers). One test that had been included, TestFTPFileSystem, is no longer included in hdfs tests. This is fine, it was in the ten-minute test.
Six new tests were added: TestFileSystem, TestNameNodeMetrics, TestSequenceFileMergeProgress, TestServiceLevelAuthorization, and TestSocketFactory. These aren't critical tests, but should be considered for inclusion later on. The test set definition will by necessity be fluid as new tests are written. Ideally, we should get all tests to have a shorter running time, which will let us squeeze more into the ten minutes. Done with patch. Created a separate file inside of src/test, commit-tests, which is the list of files to be included in the set. This file can be updated as the set's definition changes.
Updated build.xml to include a new target, run-commit-test which executes the set. Tom had suggested patch-test, but I'm afraid this would be confusing with test-patch. I had hoped to avoid code duplication within the build file for the new target: it should have same configuration as the regular, full test suite. Only the fileset definition is different. However, after playing around for quite a while, I couldn't get a good conditional setup going due to the many limitations of Ant. Since the run-test-hdfs-with-mr target also duplicates a lot of code, I'm guessing this is just the way it'll have to be. One thing I don't like is Ant/Junit doesn't complain if it can't find a test specified in the test file (not even if debug is turned on). This means we'll have to keep an active eye on changes to the tests and if they are put in the ten-minute test suite. This should be ameliorated once we move to a testing framework that allows better categorization of tests. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12412913/HDFS-458.patch against trunk revision 792310. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 52 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. -1 release audit. The applied patch generated 283 release audit warnings (more than the trunk's current 282 warnings). -1 core tests. The patch failed core unit tests. -1 contrib tests. The patch failed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/10/testReport/ This message is automatically generated. The contrib test failure is unrelated.
The core test is due the "create-c++-configure" target not existing, a problem with hudson. The release audit? I can't tell, there's a 404 on the release audit warning and when I run it locally I get gibberish... run-commit-test and run-test-hdfs have quite large amount of repeated codes. Could we create a common target for them?
Canceling patch to address Nicholas' comments.
+1 on the suite of tests to be included. Great work!!!
Updated patch to share the common code for both the regular tests and the common tests, using a macro. Thanks to Nicholas for suggesting the workaround. I don't believe there's any lose of functionality.
I am inclined to have 10 minutes tests to be collected in a JUnit's test suite instead of an external file. The reason is simple: with the file we'd have two points of maintenance: tests source code and one an auxiliary text files. Besides, a general approach is to use JUnit's suites for tagging purposes, thus having a suite here would be more uniform.
I considered that but thought that having one easy-to-edit source of tests would be a benefit. Add a test? Add its name. Delete a delete? Delete it. I think a plain text file is good for this type of info. Further, having them run in the same configuration as they would normally be run through ant via the update build file is nice as well.
That being said, once the tagging/test categorization setup is complete, I definitely want to move to using that. I'd much rather be able to prefer to be able to annotate specific tests within each test suite to be included in the 10-minute set. Right now we have to have all the tests from each suite included, rather than on a test-by-test basis. As soon as that's up and running, we should convert over. -1 overall. Here are the results of testing the latest attachment
http://issues.apache.org/jira/secure/attachment/12413892/HDFS-458.patch against trunk revision 794953. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 55 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. -1 release audit. The applied patch generated 284 release audit warnings (more than the trunk's current 282 warnings). -1 core tests. The patch failed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/25/testReport/ This message is automatically generated. Please use this link for Releaseaudit warnigns:
http://hudson.zones.apache.org/hudson/job/Hdfs-Patch-vesta.apache.org/25/artifact/trunk/patchprocess/releaseAuditDiffWarnings.txt I ve this fixed for the latest patch-test builds. OK, Hudson is completely wonky on this one.
The two new warnings are due to the files that list the tests don't have apache licenses. There's no support for this in ant. They can't have the licenses (it could conceivably work, but may screw things up later). This is OK.
Hudson is basing this on the fact that the number of tests went down from the previous patch it tested. However, that patch (
If you look at the raw console output it actually says this patch improved the number of javac warnings, which isn't possible. Again, this is ok. +1 patch looks good. I agree that the patch is ready to go.
The javac problem was reported in +1 From a QA stand point, everything has been account for. We just want to make sure there are bugs in for all the issues that were raised. (According to Jakob there are.)
Note that the patch has removed the aop dir from run-test-hdfs target. The aop related targets will be done in
- <fileset dir="${test.src.dir}/aop" - includes="**/${test.include}.java" - excludes="**/${test.exclude}.java" /> Tried "ant run-test-hdfs" and "ant run-commit-test" manually. Both worked fine. I have committed this. Thanks, Jakob! Question: Given that a patch has already passed review and test-patch, is it ready to be committed once it has passed "ant run-commit-test" (instead of "ant test")?
+1 on using 'run-commit-test' for patch verifications.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
The end result is a proposed test set that runs on median in 9.22 minutes (compared to 40 minutes for all hdfs tests) and provides 88% of the original test coverage. I think this is a pretty good result. The median is used because a fair proportion of tests have shown outlier running times. Unfortunately, our initial code coverage number of 49% is lacking.
A focus was put on maintaining as much as possible the code coverage of the major HDFS classes, including Namenode, FSNamesystem, DFSClient, etc. Many of the tests within hdfs effectively act as integration tests (particularly when a test invokes the MiniDFSCluster), making it relatively easy to identify tests that stress these classes and code paths.
Another result in the spreadsheet is the identification of several tests with very large variations in run time that should be stabilized.
The build script defines another build target, run-test-ten, which runs the tests that are defined as part of the ten-minute test. You can use this to play with other combos, if you like. However, it's a pre-split build script.
Suggestions? Comments? Snark?