----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7068/#review11437 -----------------------------------------------------------
Overall a good change, thanks for contributing a patch to fix this! Historically we've had test setup code duplicated in a bunch of places, and there have been steps toward consolidating that, and you've done a bit more of that here which is cool. Not a blocker for this patch, but if you want to go a step further consider extending HCatBaseTest and switching to junit4 style tests. http://svn.apache.org/viewvc/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/HCatBaseTest.java?view=markup http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java <https://reviews.apache.org/r/7068/#comment24479> It would be helpful for this comment to include a note that checkOutputSpecs is the call that creates the directory. Here we mention it could be created by another tasks, but I think we could clarify that the first task to reach this try block is what creates the directory. http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java <https://reviews.apache.org/r/7068/#comment24480> I haven't looked at the client cache in detail, but is there a way to disable it? If no, would it be straightforward to do so? That might be a useful property to set when running tests in general. http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java <https://reviews.apache.org/r/7068/#comment24481> What is fs.getWorkingDirectory in this case? I'm curious if this causes data to be created outside the build directory. If its not in the build directory can we move it there? I find it suuuuuper annoying that "ant clean" does not remove all generated files, which makes looking at "svn st" or "git status" cluttered. This is especially annoying when running tests before commit because I think its going to lead to forgetting to add/remove a file at some point when the output is misread. http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java <https://reviews.apache.org/r/7068/#comment24478> Can runHCatDynamicPartitionedTable be private/protected? I might have missed a call but it looks like this does not need to be exposed. - Travis Crawford On Sept. 12, 2012, 10:42 p.m., Arup Malakar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7068/ > ----------------------------------------------------------- > > (Updated Sept. 12, 2012, 10:42 p.m.) > > > Review request for hcatalog and Travis Crawford. > > > Description > ------- > > 1. Added a test case to reproduce the bug > 2. Fixed the bug > 2. Apart from some cosmetic changes for readability in HCatMapReduceTest > Removed the abstract method initialize() in HCatMapReduceTest as the > same could be achieved by using the well understood method setUp() > > > This addresses bug HCATALOG-490. > https://issues.apache.org/jira/browse/HCATALOG-490 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/java/org/apache/hcatalog/mapreduce/FileRecordWriterContainer.java > 1384153 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/HCatMapReduceTest.java > 1384153 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatDynamicPartitioned.java > 1384153 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatNonPartitioned.java > 1384153 > > http://svn.apache.org/repos/asf/incubator/hcatalog/trunk/src/test/org/apache/hcatalog/mapreduce/TestHCatPartitioned.java > 1384153 > > Diff: https://reviews.apache.org/r/7068/diff/ > > > Testing > ------- > > test: > [mkdir] Created dir: > /Users/malakar/code/oss/hcatalog/trunk/build/test/logs > [mkdir] Created dir: > /Users/malakar/code/oss/hcatalog/trunk/build/test/hcat_junit_warehouse > [junit] WARNING: multiple versions of ant detected in path for junit > [junit] > jar:file:/Users/malakar/bin/apache-ant-1.8.4/lib/ant.jar!/org/apache/tools/ant/Project.class > [junit] and > jar:file:/Users/malakar/code/oss/hcatalog/trunk/build/ivy/lib/test/ant-ant-1.6.5.jar!/org/apache/tools/ant/Project.class > [junit] Running org.apache.hcatalog.cli.TestPermsGrp > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 14.854 sec > [junit] Running org.apache.hcatalog.cli.TestSemanticAnalysis > [junit] Tests run: 22, Failures: 0, Errors: 0, Time elapsed: 20.615 sec > [junit] Running org.apache.hcatalog.cli.TestUseDatabase > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 13.533 sec > [junit] Running org.apache.hcatalog.common.TestHCatUtil > [junit] Tests run: 5, Failures: 0, Errors: 0, Time elapsed: 0.868 sec > [junit] Running org.apache.hcatalog.common.TestHiveClientCache > [junit] Tests run: 6, Failures: 0, Errors: 0, Time elapsed: 38.042 sec > [junit] Shutting down hive metastore. > [junit] Running org.apache.hcatalog.data.TestDefaultHCatRecord > [junit] Tests run: 5, Failures: 0, Errors: 0, Time elapsed: 0.21 sec > [junit] Running org.apache.hcatalog.data.TestHCatRecordSerDe > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.181 sec > [junit] Running org.apache.hcatalog.data.TestJsonSerDe > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 0.239 sec > [junit] Running org.apache.hcatalog.data.TestLazyHCatRecord > [junit] Tests run: 11, Failures: 0, Errors: 0, Time elapsed: 0.177 sec > [junit] Running org.apache.hcatalog.data.TestReaderWriter > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 5.083 sec > [junit] Running org.apache.hcatalog.data.schema.TestHCatSchema > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 0.007 sec > [junit] Running org.apache.hcatalog.data.schema.TestHCatSchemaUtils > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.043 sec > [junit] Running org.apache.hcatalog.listener.TestMsgBusConnection > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 7.588 sec > [junit] Running org.apache.hcatalog.listener.TestNotificationListener > [junit] Tests run: 1, Failures: 1, Errors: 0, Time elapsed: 8.045 sec > [junit] Test org.apache.hcatalog.listener.TestNotificationListener FAILED > [junit] Running org.apache.hcatalog.mapreduce.TestHCatDynamicPartitioned > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 122.809 sec > [junit] Running org.apache.hcatalog.mapreduce.TestHCatHiveCompatibility > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 32.425 sec > [junit] Running > org.apache.hcatalog.mapreduce.TestHCatHiveThriftCompatibility > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 11.662 sec > [junit] Running org.apache.hcatalog.mapreduce.TestHCatInputFormat > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 10.985 sec > [junit] Running org.apache.hcatalog.mapreduce.TestHCatMultiOutputFormat > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 28.85 sec > [junit] Shutting down hive metastore. > [junit] Running org.apache.hcatalog.mapreduce.TestHCatNonPartitioned > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 14.169 sec > [junit] Running org.apache.hcatalog.mapreduce.TestHCatOutputFormat > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 5.323 sec > [junit] Running org.apache.hcatalog.mapreduce.TestHCatPartitioned > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 92.827 sec > [junit] Running org.apache.hcatalog.mapreduce.TestMultiOutputFormat > [junit] Tests run: 2, Failures: 0, Errors: 0, Time elapsed: 67.213 sec > [junit] Running org.apache.hcatalog.mapreduce.TestPassProperties > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 8.611 sec > [junit] Running org.apache.hcatalog.mapreduce.TestSequenceFileReadWrite > [junit] Tests run: 4, Failures: 0, Errors: 0, Time elapsed: 53.583 sec > [junit] Running org.apache.hcatalog.rcfile.TestRCFileMapReduceInputFormat > [junit] Tests run: 1, Failures: 0, Errors: 0, Time elapsed: 0.216 sec > [junit] Running org.apache.hcatalog.security.TestHdfsAuthorizationProvider > [junit] Tests run: 34, Failures: 0, Errors: 0, Time elapsed: 23.476 sec > > > Thanks, > > Arup Malakar > >
