[ https://issues.apache.org/jira/browse/HADOOP-11746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14499986#comment-14499986 ]
Sean Busbey commented on HADOOP-11746: -------------------------------------- +1 after non-nit issues below. Adding audience / stability comments is a really nice touch. {code} +## @description Some crazy people like Eclipse. Make sure Maven's eclipse generation +## @description works so they don't freak out. {code} Please leave out the ableist pejorative. {code} +function check_unittests +{ + verify_needed_test unit + + if [[ $? == 0 ]]; then + echo "Existing unit tests do not test patched files. Skipping." + return 0 + fi + big_console_header "Running unit tests" {code} Nit: could we make this (and maybe the module choosing) optional so that if I know tests end up covering my changes indirectly I can set a flag to run through things? (Although maybe I missed how I could do this myself) This will mostly be useful when I try to merge this set of changes with things over in HBase, since we run through all the unit tests on pre-patch. {code} + if [[ -d "${mypwd}/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs" ]]; then + echo "Changing permission on ${mypwd}/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs to avoid broken builds" + chmod +x -R "${mypwd}/hadoop-hdfs-project/hadoop-hdfs/target/test/data/dfs" + fi {code} Nit: so long as we're cleaning things up, could we replace this with a general solution? The root cause of the issue that introduced this change originally was that we need to ensure any directories that exist under a maven module target directory are executable. We know tests can break that one, but there's no reason some other module might break a different one later. > rewrite test-patch.sh > --------------------- > > Key: HADOOP-11746 > URL: https://issues.apache.org/jira/browse/HADOOP-11746 > Project: Hadoop Common > Issue Type: Test > Components: build, test > Affects Versions: 3.0.0 > Reporter: Allen Wittenauer > Assignee: Allen Wittenauer > Attachments: HADOOP-11746-00.patch, HADOOP-11746-01.patch, > HADOOP-11746-02.patch, HADOOP-11746-03.patch, HADOOP-11746-04.patch, > HADOOP-11746-05.patch, HADOOP-11746-06.patch, HADOOP-11746-07.patch, > HADOOP-11746-09.patch, HADOOP-11746-10.patch, HADOOP-11746-11.patch, > HADOOP-11746-12.patch, HADOOP-11746-13.patch, HADOOP-11746-14.patch, > HADOOP-11746-15.patch, HADOOP-11746-16.patch, HADOOP-11746-17.patch > > > This code is bad and you should feel bad. -- This message was sent by Atlassian JIRA (v6.3.4#6332)