hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Nauroth (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HADOOP-11746) rewrite test-patch.sh
Date Wed, 15 Apr 2015 19:21:59 GMT

    [ https://issues.apache.org/jira/browse/HADOOP-11746?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14496745#comment-14496745
] 

Chris Nauroth commented on HADOOP-11746:
----------------------------------------

Thank you for the patch, Allen.  The new functionality looks great!  Here are a few comments.

checkstyle.sh
# {{checkstyle_postapply}}: Please correct the indentation on this line.
{code}
 cp -p "${BASEDIR}/target/checkstyle-result.xml" "${PATCH_DIR}/checkstyle-result-patch.xml"
{code}
# Do you think it's worthwhile to move the Python code into its own .py file?

shellcheck.sh
# {{shellcheck_private_findbash}}: This looks for files in bin and sbin.  Do we also need
libexec, which is currently used in hadoop-kms and hadoop-hdfs-httpfs?
# There are shellcheck warnings on lines 49 and 70.  I see shellcheck.sh caught them in the
last run, so that's cool.  :-)

whitespace.sh
# From some quick testing of the grep, it's catching trailing spaces, but not trailing tabs.

test-patch.sh
# {{precheck_without_patch}} and {{check_site}}: The mvn site command that is echoed is slightly
different from what is actually run.  The arguments are in a different order.  I wonder if
it would be helpful to have a {{echo_and_exec}} helper function to call everywhere that we
do this kind of thing.
# {{determine_needed_tests}}: I believe this would not identify tests for a patch that contained
only changes in test resources (not Java test code).  Examples of this include testConf.xml
and editsStored and editsStored.xml.
# {{check_unittests}}: The echo of the mvn command on line 1736 does not include the redirection
of output to test_logfile as the actual executed command does.
# {{output_to_console}}: Today I learned that your secret talent is ASCII elephant art.  :-)
# There are shellcheck warnings on lines 1158, 1573, 1732, 1751, and 1995.
# The old test-patch.sh output always included a hyperlink to the patch file that it ran.
 Can we please add that?  I always found that helpful for knowing exactly which patch it was
reporting on, in case multiple revisions got uploaded quickly.

Are you planning to clean up smart-apply-patch.sh in scope of this jira, or will that happen
elsewhere?


> 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
>
>
> This code is bad and you should feel bad.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message