> On Sept. 16, 2013, 4:04 p.m., Rohini Palaniswamy wrote: > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java, > > line 402 > > <https://reviews.apache.org/r/13674/diff/1/?file=342319#file342319line402> > > > > Wouldn't File.Separator be better instead of "/" ?
This isn't a File separator, it's a URI path separator. It's always going to be "/". As far as I could tell, there is nothing like URI.Separator that we can use. > On Sept. 16, 2013, 4:04 p.m., Rohini Palaniswamy wrote: > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java, > > line 813 > > <https://reviews.apache.org/r/13674/diff/1/?file=342303#file342303line813> > > > > Same as before. Can pass File. We should move this method to XTestCase > > as it is getting repeated in lot of testcases. there's already a CreateTestCaseSubDir method, so I replaced all of these with lines like this: createTestCaseSubDir("2009/09/09/_SUCCESS".split("/")); > On Sept. 16, 2013, 4:04 p.m., Rohini Palaniswamy wrote: > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java, > > lines 59-60 > > <https://reviews.apache.org/r/13674/diff/1/?file=342301#file342301line59> > > > > Why not getTestCaseFileUri in both places? Would be better to move that > > out as a variable. Confusing to see one new File(getTestCaseDir(),..) and > > another getTestCaseFileUri > > > > Same for other methods in this class. Changed to look like this: String workflowUri = getTestCaseFileUri("workflow.xml"); writeToFile(appXml, workflowUri); conf.set(OozieClient.APP_PATH, workflowUri); - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13674/#review26138 ----------------------------------------------------------- On Aug. 19, 2013, 10:31 p.m., David Wannemacher wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13674/ > ----------------------------------------------------------- > > (Updated Aug. 19, 2013, 10:31 p.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1500 > https://issues.apache.org/jira/browse/OOZIE-1500 > > > Repository: oozie > > > Description > ------- > > This is a large patch, please let me know if I should split it up and/or use > the code reviewer tool. I tested it on linux and verified that it did not > cause any unit test regressions. > > This patch fixes many unix-specific assumptions in code and tests: > 1. Using "/" as a path separator. On windows, the right separator is "\". > Fix: create a new Path object with the directory and subdirectory names, > which handles using the right separator. > 2. Determining whether a path is absolute by testing if it begins with "/". > Fix: Create a new File object and use isAbsolute(). > 3. Generating a pathname by concatenating "file://", getTestCaseDir(), > File.separator, and the file name. This is wrong because this is a URI, and > File.separator is "/" on unix but "\" on windows. Fix: New helper method > getTestCaseFileUri solves this. > 4. Create directories by using "mkdir" shell command with hard-coded file > separators. Fix: Create a new File object and use mkdirs(). > 5. Scripts that use unix-only commands (ls, echo, etc.). Fix: Implement > windows-specific commands and use Shell.WINDOWS to pick the right one at > runtime. > 6. Assume that perl exists in TestShellActionExecutor.testPerlScript. It > doesn't on Windows. Fix: skip the test on Windows. > 7. Use the test case name as part of the temp directory name in order to > ensure directories are unique. This can cause issues on Windows. Fix: Use a > random UUID instead. > 8. As a result of using getTestCaseUri, some test case verification that uses > string equality fails. (TestLiteWorkflowAppService) Fix: use URI equivalence > instead of string equality. > 9. AdminServlet tests verify that system variables are working by checking > for a USER variable. This is called USERNAME in windows. Fix: On Windows, > check for USERNAME instead. > 10. TestHostnameFilter checks that "localhost" is in the filter. On Windows, > this is "127.0.0.1". Fix: On Windows, check for 127.0.0.1 instead. > > On windows there are still several tests failing. I hope to spend some time > fixing these later: > Failed tests: > testActionCheck(org.apache.oozie.command.wf.TestActionCheckXCommand) > testActionCheckTransientDuringLauncher(org.apache.oozie.command.wf.TestActionCheckXCommand) > testActionCheckTransientDuringMRAction(org.apache.oozie.command.wf.TestActionCheckXCommand) > testCoordinatorActionCommands(org.apache.oozie.sla.TestSLAEventGeneration): > expected:<NOT_STARTED> but was:<IN_PROCESS> > testChmodWithGlob(org.apache.oozie.action.hadoop.TestFsActionExecutor): > expected:<rw[-]------> but was:<rw[x]------> > Tests in error: > testWaitFor(org.apache.oozie.test.TestXTestCase): could not delete path > [D:\apache\oozie\core\target\test-data\oozietests\org.apache.oozie.test.TestXTestCase$MyXTestCase\testWaitFor\conf\oozie-site.xml] > testBaseDir(org.apache.oozie.test.TestXTestCase): could not delete path > [D:\apache\oozie\core\target\test-data\oozietests\org.apache.oozie.test.TestXTestCase$MyXTestCase\testBaseDir\conf\oozie-site.xml] > > How to run unit tests in windows: > 1. You will need to build hadoop-core from branch-1-win and install it into > your local maven cache. > 2. In the root pom.xml, replace the dummy value in HADOOP_HOME with the path > to the hadoop directory. The minicluster tests require bin\winutils.exe to be > in this directory. > 3. In the root pom.xml (and hadooplibs version 1 pom.xmls, if OOZIE-1490 is > not committed yet) change the hadoop version to the version built from > branch-1-win. > 4. May also need to use Hive 0.11 with the windows fixes. > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/service/CallbackService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/service/ConfigurationService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/main/java/org/apache/oozie/service/Services.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngineStreamLog.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/TestDagEngine.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/ShellTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestRerun.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/client/TestLocalOozie.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionStartXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordCommandUtils.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestFutureActionsTimeOut.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/coord/TestPastActionsTimeOut.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionErrors.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestLastModified.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestReRunXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestSignalXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/coord/TestCoordELEvaluator.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/coord/TestCoordELFunctions.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/event/TestEventGeneration.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordActionGetForInputCheckJPAExecutor.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestActionCheckerService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestAuthorizationService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestLiteWorkflowAppService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestPurgeService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/servlet/TestAdminServlet.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/servlet/TestHostnameFilter.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/servlet/TestV1AdminServlet.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/TestXFsTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/XDataTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/XFsTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/XHCatTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/core/src/test/java/org/apache/oozie/test/XTestCase.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/examples/src/main/java/org/apache/oozie/example/LocalOozieExample.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/minitest/src/test/java/org/apache/oozie/test/WorkflowTest.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/PipesMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/pig/src/main/java/org/apache/oozie/action/hadoop/PigMainWithOldAPI.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/sqoop/src/main/java/org/apache/oozie/action/hadoop/SqoopMain.java > 1515521 > > http://svn.apache.org/repos/asf/oozie/trunk/sharelib/streaming/src/main/java/org/apache/oozie/action/hadoop/StreamingMain.java > 1515521 > > Diff: https://reviews.apache.org/r/13674/diff/ > > > Testing > ------- > > Ran unit tests on Windows and Linux. > > > Thanks, > > David Wannemacher > >
