[SPARK-12508][PROJECT-INFRA] Fix minor bugs in dev/tests/pr_public_classes.sh script
This patch fixes a handful of minor bugs in the `dev/tests/pr_public_classes.sh` script, which is used by the `run_tests_jenkins` script to detect the addition of new public classes: - Account for differences between BSD and GNU `sed` in order to allow the script to run on OS X. - Diff `$ghprbActualCommit^...$ghprbActualCommit ` instead of `master...$ghprbActualCommit`: since `ghprbActualCommit` is a merge commit which results from merging the PR into the target branch, this will give us the desired diff and will avoid certain race-conditions which could lead to false-positives. - Use `echo -e` instead of `echo` so that newline characters are handled correctly in output. This should fix a formatting glitch which caused the output to appear on a single line in the GitHub comment (see [the SC2028 page](https://github.com/koalaman/shellcheck/wiki/SC2028) on the Shellcheck wiki for more details). Author: Josh Rosen <joshrosen@databricks.com> Closes #10455 from JoshRosen/fix-pr-public-classes-test.
This commit is contained in:
parent
8e23d8db7f
commit
ab6bedd85d
|
@ -24,36 +24,44 @@
|
|||
#
|
||||
# Arg1: The Github Pull Request Actual Commit
|
||||
#+ known as `ghprbActualCommit` in `run-tests-jenkins`
|
||||
# Arg2: The SHA1 hash
|
||||
#+ known as `sha1` in `run-tests-jenkins`
|
||||
#
|
||||
|
||||
# We diff master...$ghprbActualCommit because that gets us changes introduced in the PR
|
||||
#+ and not anything else added to master since the PR was branched.
|
||||
|
||||
ghprbActualCommit="$1"
|
||||
sha1="$2"
|
||||
|
||||
# $ghprbActualCommit is an automatic merge commit generated by GitHub; its parents are some Spark
|
||||
# master commit and the tip of the pull request branch.
|
||||
|
||||
# By diffing$ghprbActualCommit^...$ghprbActualCommit and filtering to examine the diffs of only
|
||||
# non-test files, we can gets us changes introduced in the PR and not anything else added to master
|
||||
# since the PR was branched.
|
||||
|
||||
# Handle differences between GNU and BSD sed
|
||||
if [[ $(uname) == "Darwin" ]]; then
|
||||
SED='sed -E'
|
||||
else
|
||||
SED='sed -r'
|
||||
fi
|
||||
|
||||
source_files=$(
|
||||
git diff master...$ghprbActualCommit --name-only `# diff patch against master from branch point` \
|
||||
git diff $ghprbActualCommit^...$ghprbActualCommit --name-only `# diff patch against master from branch point` \
|
||||
| grep -v -e "\/test" `# ignore files in test directories` \
|
||||
| grep -e "\.py$" -e "\.java$" -e "\.scala$" `# include only code files` \
|
||||
| tr "\n" " "
|
||||
)
|
||||
|
||||
new_public_classes=$(
|
||||
git diff master...$ghprbActualCommit ${source_files} `# diff patch against master from branch point` \
|
||||
git diff $ghprbActualCommit^...$ghprbActualCommit ${source_files} `# diff patch against master from branch point` \
|
||||
| grep "^\+" `# filter in only added lines` \
|
||||
| sed -r -e "s/^\+//g" `# remove the leading +` \
|
||||
| $SED -e "s/^\+//g" `# remove the leading +` \
|
||||
| grep -e "trait " -e "class " `# filter in lines with these key words` \
|
||||
| grep -e "{" -e "(" `# filter in lines with these key words, too` \
|
||||
| grep -v -e "\@\@" -e "private" `# exclude lines with these words` \
|
||||
| grep -v -e "^// " -e "^/\*" -e "^ \* " `# exclude comment lines` \
|
||||
| sed -r -e "s/\{.*//g" `# remove from the { onwards` \
|
||||
| sed -r -e "s/\}//g" `# just in case, remove }; they mess the JSON` \
|
||||
| sed -r -e "s/\"/\\\\\"/g" `# escape double quotes; they mess the JSON` \
|
||||
| sed -r -e "s/^(.*)$/\`\1\`/g" `# surround with backticks for style` \
|
||||
| sed -r -e "s/^/ \* /g" `# prepend ' *' to start of line` \
|
||||
| sed -r -e "s/$/\\\n/g" `# append newline to end of line` \
|
||||
| $SED -e "s/\{.*//g" `# remove from the { onwards` \
|
||||
| $SED -e "s/\}//g" `# just in case, remove }; they mess the JSON` \
|
||||
| $SED -e "s/\"/\\\\\"/g" `# escape double quotes; they mess the JSON` \
|
||||
| $SED -e "s/^(.*)$/\`\1\`/g" `# surround with backticks for style` \
|
||||
| $SED -e "s/^/ \* /g" `# prepend ' *' to start of line` \
|
||||
| $SED -e "s/$/\\\n/g" `# append newline to end of line` \
|
||||
| tr -d "\n" `# remove actual LF characters`
|
||||
)
|
||||
|
||||
|
@ -61,5 +69,5 @@ if [ -z "$new_public_classes" ]; then
|
|||
echo " * This patch adds no public classes."
|
||||
else
|
||||
public_classes_note=" * This patch adds the following public classes _(experimental)_:"
|
||||
echo "${public_classes_note}\n${new_public_classes}"
|
||||
echo -e "${public_classes_note}\n${new_public_classes}"
|
||||
fi
|
||||
|
|
Loading…
Reference in a new issue