hadoop-common-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Allen Wittenauer (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (HADOOP-11746) rewrite test-patch.sh
Date Thu, 16 Apr 2015 08:41:59 GMT

     [ https://issues.apache.org/jira/browse/HADOOP-11746?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Allen Wittenauer updated HADOOP-11746:
--------------------------------------
    Attachment: HADOOP-11746-16.patch

-16:
* Fixed, etc through all of the [~cnauroth]'s review comments.  Much thanks!

Some notes:

{{checkstyle.sh}}

bq. 2. Do you think it's worthwhile to move the Python code into its own .py file?

I've been debating this myself.  Two advantages of doing it this way:
* the whole plug-in is self contained.
* the dev-support patch detection is a lot easier

{{shellcheck.sh}}

bq. 1. {{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?

Great catch.  I also added shellprofile.d while I was there!

{{whitespace.sh}}

Ugh. Good point. That should be [[:blank:]] not [[:space:]].  Easy fix.  This also means this
works in non-C locales a bit better.

{{testpatch.sh}}

bq. 2. {{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.

Relevant code:
{code}
    elif [[ ${i} =~ \.c$
...
      || ${i} =~ src/test
...
       ]]; then
       hadoop_debug "tests/native: ${i}"
       add_test javac
       add_test unit
{code}

It should.  If *any* file has src/test in its path, it will trigger the javac and unit tests.
 It's really not "tests/native", so maybe that should get renamed to something else I guess.
 (This is counter to tests/java which also triggers javadoc tests)

bq. 4. {{output_to_console}}: Today I learned that your secret talent is ASCII elephant art.


I don't know what you are talking about.  Maybe your patch download was bad? ;)

bq. 5. There are shellcheck warnings on lines 1158, 1573, 1732, 1751, and 1995.

Fixed or disabled all of these except two, which are a) false positives and b) extremely hard
to escape because they are actually awk commands in a multiline pipe.

bq. 6. 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.

It actually does this when the patch provided is from either a JIRA or a URL.  This is consistent
with the old code.

FWIW, I've uploaded the same patch into HADOOP-11820 and ran it in jenkins mode.  You'll see
the shellcheck #'s and the patch URL there as well. Check out that execution time. :D :D

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



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

Mime
View raw message