You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by fpj <gi...@git.apache.org> on 2016/10/31 12:15:01 UTC

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

GitHub user fpj opened a pull request:

    https://github.com/apache/zookeeper/pull/97

    ZOOKEEPER-2624: Add test script for pull requests

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/fpj/zookeeper ZK-2624

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/97.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #97
    
----
commit 1929614a7ef076d9e2bd1191b3341207f2e4411e
Author: fpj <fp...@apache.org>
Date:   2016-10-31T12:13:59Z

    Initial cut of the script and changes to build.xml

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86679330
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,612 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ## Obtain PR number and title
    +      PULLREQUEST_ID=${GIT_PR_NUMBER}
    +      PULLREQUEST_TITLE="${GIT_PR_TITLE}"
    +
    +      ## Extract jira number from PR title
    +      defect=${PULLREQUEST_TITLE%%:*}
    +
    +      echo "Pull request id: ${PULLREQUEST_ID}"
    +      echo "Pull request title: ${PULLREQUEST_TITLE}"
    +      echo "Defect number: ${defect}"
    +      JIRA_COMMENT="GitHub Pull Request ${PULLREQUEST_NUMBER} Build
    +      "
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <GIT_PR_URL> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      PATCH_FILE=${PATCH_DIR}/patch
    +      curl -L $2.diff > ${PATCH_FILE}
    +      ### PATCH_FILE contains the location of the patchfile
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      local subject=`grep "Subject:" ${PATCH_FILE}`
    +      local length=`expr match ${subject} ZOOKEEPER-[0-9]*`
    +      local position=`expr index ${subject} ZOOKEEPER-`
    +      defect=${${subject:$position:$length}#ZOOKEEPER-}
    +      ;;
    +    *)
    +      echo "ERROR: usage $0 QABUILD [args] | DEVELOPER [args]"
    +      cleanupAndExit 0
    +      ;;
    +  esac
    +}
    +
    +###############################################################################
    +checkout () {
    +  echo ""
    +  echo ""
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo "    Testing patch for pull request ${PULLREQUEST_ID}."
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo ""
    +  echo ""
    +  ### When run by a developer, if the workspace contains modifications, do not continue
    +  # Ref http://stackoverflow.com/a/2659808 for details on checking dirty status
    +  ${GIT} diff-index --quiet HEAD
    +  if [[ $? -ne 0 ]] ; then
    +    uncommitted=`${GIT} diff --name-only HEAD`
    +    uncommitted="You have the following files with uncommitted changes:${NEWLINE}${uncommitted}"
    +  fi
    +  untracked="$(${GIT} ls-files --exclude-standard --others)" && test -z "${untracked}"
    +  if [[ $? -ne 0 ]] ; then
    +    untracked="You have untracked and unignored files:${NEWLINE}${untracked}"
    +  fi
    +
    +  if [[ $QABUILD == "false" ]] ; then
    +    if [[ $uncommitted || $untracked ]] ; then
    +      echo "ERROR: can't run in a workspace that contains the following modifications"
    +      echo ""
    +      echo "${uncommitted}"
    +      echo ""
    +      echo "${untracked}"
    +      cleanupAndExit 1
    +    fi
    +  else
    +    # I don't believe we need to do anything here - the jenkins job will
    +    # cleanup the environment for us ("cleanup before checkout" action)
    +    # on the precommit jenkins job
    +    echo
    +  fi
    +  return $?
    +}
    +
    +###############################################################################
    +setup () {
    +  ### exit if warnings are NOT defined in the properties file
    +  if [ -z "$OK_FINDBUGS_WARNINGS" ] || [[ -z "$OK_JAVADOC_WARNINGS" ]] || [[ -z $OK_RELEASEAUDIT_WARNINGS ]]; then
    --- End diff --
    
    slight inconsistency with [ vs [[


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by breed <gi...@git.apache.org>.
Github user breed commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    agreed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by rgs1 <gi...@git.apache.org>.
Github user rgs1 commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    @hanm @fpj i say we merge it and keep iterating; it'll be easier to get it right once we are using it. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86384484
  
    --- Diff: build.xml ---
    @@ -1622,6 +1623,26 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
         	</exec>
          </target>
     
    +     <target name="qa-test-pullrequest" depends="findbugs.check,forrest.check">
    +        <exec executable="bash" failonerror="true">
    +                <arg value="${test_pullrequest_sh}"/>
    +                <arg value="QABUILD"/>
    +                <arg value="${scratch.dir}"/>
    +                <arg value="${ps.cmd}"/>
    +                <arg value="${wget.cmd}"/>
    +                <arg value="${jiracli.cmd}"/>
    +                <arg value="${git.cmd}"/>
    +                <arg value="${grep.cmd}"/>
    +                <arg value="${patch.cmd}"/>
    +                <arg value="${findbugs.home}"/>
    +                <arg value="${forrest.home}"/>
    +                <arg value="${basedir}"/>
    +                <arg value="${jira.passwd}"/>
    +                <arg value="${java5.home}"/>
    +                <arg value="${curl.cmd}"/>
    --- End diff --
    
    There was a '<arg value="${defect}"/>' in test-patch.sh but missing here - is this intentionally left out? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    The failures are the same as before: `@author` string in the script and new findbugs warnings due to jenkins upgrade. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by rgs1 <gi...@git.apache.org>.
Github user rgs1 commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86679093
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,605 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.  
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "Number of given parameters: $#" 
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    --- End diff --
    
    nit: use `[[ ! -e "$PATCH_DIR" ]]` as you do in other places  


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86679336
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,612 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ## Obtain PR number and title
    +      PULLREQUEST_ID=${GIT_PR_NUMBER}
    +      PULLREQUEST_TITLE="${GIT_PR_TITLE}"
    +
    +      ## Extract jira number from PR title
    +      defect=${PULLREQUEST_TITLE%%:*}
    +
    +      echo "Pull request id: ${PULLREQUEST_ID}"
    +      echo "Pull request title: ${PULLREQUEST_TITLE}"
    +      echo "Defect number: ${defect}"
    +      JIRA_COMMENT="GitHub Pull Request ${PULLREQUEST_NUMBER} Build
    +      "
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <GIT_PR_URL> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      PATCH_FILE=${PATCH_DIR}/patch
    +      curl -L $2.diff > ${PATCH_FILE}
    +      ### PATCH_FILE contains the location of the patchfile
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      local subject=`grep "Subject:" ${PATCH_FILE}`
    +      local length=`expr match ${subject} ZOOKEEPER-[0-9]*`
    +      local position=`expr index ${subject} ZOOKEEPER-`
    +      defect=${${subject:$position:$length}#ZOOKEEPER-}
    +      ;;
    +    *)
    +      echo "ERROR: usage $0 QABUILD [args] | DEVELOPER [args]"
    +      cleanupAndExit 0
    +      ;;
    +  esac
    +}
    +
    +###############################################################################
    +checkout () {
    +  echo ""
    +  echo ""
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo "    Testing patch for pull request ${PULLREQUEST_ID}."
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo ""
    +  echo ""
    +  ### When run by a developer, if the workspace contains modifications, do not continue
    +  # Ref http://stackoverflow.com/a/2659808 for details on checking dirty status
    +  ${GIT} diff-index --quiet HEAD
    +  if [[ $? -ne 0 ]] ; then
    +    uncommitted=`${GIT} diff --name-only HEAD`
    +    uncommitted="You have the following files with uncommitted changes:${NEWLINE}${uncommitted}"
    +  fi
    +  untracked="$(${GIT} ls-files --exclude-standard --others)" && test -z "${untracked}"
    +  if [[ $? -ne 0 ]] ; then
    +    untracked="You have untracked and unignored files:${NEWLINE}${untracked}"
    +  fi
    +
    +  if [[ $QABUILD == "false" ]] ; then
    +    if [[ $uncommitted || $untracked ]] ; then
    +      echo "ERROR: can't run in a workspace that contains the following modifications"
    +      echo ""
    +      echo "${uncommitted}"
    +      echo ""
    +      echo "${untracked}"
    +      cleanupAndExit 1
    +    fi
    +  else
    +    # I don't believe we need to do anything here - the jenkins job will
    +    # cleanup the environment for us ("cleanup before checkout" action)
    +    # on the precommit jenkins job
    +    echo
    +  fi
    +  return $?
    +}
    +
    +###############################################################################
    +setup () {
    +  ### exit if warnings are NOT defined in the properties file
    +  if [ -z "$OK_FINDBUGS_WARNINGS" ] || [[ -z "$OK_JAVADOC_WARNINGS" ]] || [[ -z $OK_RELEASEAUDIT_WARNINGS ]]; then
    +    echo "Please define the following properties in test-patch.properties file"
    +	 echo  "OK_FINDBUGS_WARNINGS"
    --- End diff --
    
    what's going on with indentation?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    The failure is actually expected because the script contains the `@author` string.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86679277
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,612 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ## Obtain PR number and title
    +      PULLREQUEST_ID=${GIT_PR_NUMBER}
    +      PULLREQUEST_TITLE="${GIT_PR_TITLE}"
    +
    +      ## Extract jira number from PR title
    +      defect=${PULLREQUEST_TITLE%%:*}
    +
    +      echo "Pull request id: ${PULLREQUEST_ID}"
    +      echo "Pull request title: ${PULLREQUEST_TITLE}"
    +      echo "Defect number: ${defect}"
    +      JIRA_COMMENT="GitHub Pull Request ${PULLREQUEST_NUMBER} Build
    +      "
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <GIT_PR_URL> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      PATCH_FILE=${PATCH_DIR}/patch
    +      curl -L $2.diff > ${PATCH_FILE}
    +      ### PATCH_FILE contains the location of the patchfile
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      local subject=`grep "Subject:" ${PATCH_FILE}`
    +      local length=`expr match ${subject} ZOOKEEPER-[0-9]*`
    +      local position=`expr index ${subject} ZOOKEEPER-`
    +      defect=${${subject:$position:$length}#ZOOKEEPER-}
    +      ;;
    +    *)
    +      echo "ERROR: usage $0 QABUILD [args] | DEVELOPER [args]"
    +      cleanupAndExit 0
    +      ;;
    +  esac
    +}
    +
    +###############################################################################
    +checkout () {
    +  echo ""
    +  echo ""
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo "    Testing patch for pull request ${PULLREQUEST_ID}."
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo ""
    +  echo ""
    +  ### When run by a developer, if the workspace contains modifications, do not continue
    +  # Ref http://stackoverflow.com/a/2659808 for details on checking dirty status
    +  ${GIT} diff-index --quiet HEAD
    +  if [[ $? -ne 0 ]] ; then
    +    uncommitted=`${GIT} diff --name-only HEAD`
    +    uncommitted="You have the following files with uncommitted changes:${NEWLINE}${uncommitted}"
    +  fi
    +  untracked="$(${GIT} ls-files --exclude-standard --others)" && test -z "${untracked}"
    +  if [[ $? -ne 0 ]] ; then
    +    untracked="You have untracked and unignored files:${NEWLINE}${untracked}"
    +  fi
    +
    +  if [[ $QABUILD == "false" ]] ; then
    +    if [[ $uncommitted || $untracked ]] ; then
    +      echo "ERROR: can't run in a workspace that contains the following modifications"
    +      echo ""
    +      echo "${uncommitted}"
    +      echo ""
    +      echo "${untracked}"
    +      cleanupAndExit 1
    +    fi
    +  else
    +    # I don't believe we need to do anything here - the jenkins job will
    +    # cleanup the environment for us ("cleanup before checkout" action)
    +    # on the precommit jenkins job
    +    echo
    --- End diff --
    
    is this echo here just to get around the syntax error? if so you can use : instead



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86679319
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,612 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ## Obtain PR number and title
    +      PULLREQUEST_ID=${GIT_PR_NUMBER}
    +      PULLREQUEST_TITLE="${GIT_PR_TITLE}"
    +
    +      ## Extract jira number from PR title
    +      defect=${PULLREQUEST_TITLE%%:*}
    +
    +      echo "Pull request id: ${PULLREQUEST_ID}"
    +      echo "Pull request title: ${PULLREQUEST_TITLE}"
    +      echo "Defect number: ${defect}"
    +      JIRA_COMMENT="GitHub Pull Request ${PULLREQUEST_NUMBER} Build
    +      "
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <GIT_PR_URL> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      PATCH_FILE=${PATCH_DIR}/patch
    +      curl -L $2.diff > ${PATCH_FILE}
    +      ### PATCH_FILE contains the location of the patchfile
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      local subject=`grep "Subject:" ${PATCH_FILE}`
    +      local length=`expr match ${subject} ZOOKEEPER-[0-9]*`
    +      local position=`expr index ${subject} ZOOKEEPER-`
    +      defect=${${subject:$position:$length}#ZOOKEEPER-}
    +      ;;
    +    *)
    +      echo "ERROR: usage $0 QABUILD [args] | DEVELOPER [args]"
    +      cleanupAndExit 0
    +      ;;
    +  esac
    +}
    +
    +###############################################################################
    +checkout () {
    +  echo ""
    +  echo ""
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo "    Testing patch for pull request ${PULLREQUEST_ID}."
    +  echo "======================================================================"
    +  echo "======================================================================"
    +  echo ""
    +  echo ""
    +  ### When run by a developer, if the workspace contains modifications, do not continue
    +  # Ref http://stackoverflow.com/a/2659808 for details on checking dirty status
    +  ${GIT} diff-index --quiet HEAD
    +  if [[ $? -ne 0 ]] ; then
    +    uncommitted=`${GIT} diff --name-only HEAD`
    +    uncommitted="You have the following files with uncommitted changes:${NEWLINE}${uncommitted}"
    +  fi
    +  untracked="$(${GIT} ls-files --exclude-standard --others)" && test -z "${untracked}"
    +  if [[ $? -ne 0 ]] ; then
    +    untracked="You have untracked and unignored files:${NEWLINE}${untracked}"
    +  fi
    +
    +  if [[ $QABUILD == "false" ]] ; then
    +    if [[ $uncommitted || $untracked ]] ; then
    +      echo "ERROR: can't run in a workspace that contains the following modifications"
    +      echo ""
    +      echo "${uncommitted}"
    +      echo ""
    +      echo "${untracked}"
    +      cleanupAndExit 1
    +    fi
    +  else
    +    # I don't believe we need to do anything here - the jenkins job will
    +    # cleanup the environment for us ("cleanup before checkout" action)
    +    # on the precommit jenkins job
    +    echo
    +  fi
    +  return $?
    --- End diff --
    
    i think this will always be 0, so you probably don't even need to return it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    @hanm The `DEVELOPER` mode is for running locally, so initially I left it mostly unchanged and the original script takes a patch file to test. Because of your feedback, I thought that it might be better to take a pull request URL instead to test, so I changed the script for the developer mode a bit to do it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86386241
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,607 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <PATCH_FILE> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      ### PATCH_FILE contains the location of the patchfile
    +      PATCH_FILE=$2
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      defect=`basename $PATCH_FILE`
    --- End diff --
    
    not really, we care about it to write the report to the jira.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by hanm <gi...@git.apache.org>.
Github user hanm commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86386121
  
    --- Diff: src/java/test/bin/test-github-pr.sh ---
    @@ -0,0 +1,607 @@
    +#!/usr/bin/env bash
    +#   Licensed under the Apache License, Version 2.0 (the "License");
    +#   you may not use this file except in compliance with the License.
    +#   You may obtain a copy of the License at
    +#
    +#       http://www.apache.org/licenses/LICENSE-2.0
    +#
    +#   Unless required by applicable law or agreed to in writing, software
    +#   distributed under the License is distributed on an "AS IS" BASIS,
    +#   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    +#   See the License for the specific language governing permissions and
    +#   limitations under the License.
    +
    +
    +#set -x
    +
    +### Setup some variables.
    +### GIT_COMMIT and BUILD_URL are set by Hudson if it is run by patch process
    +### Read variables from properties file
    +. `dirname $0`/test-patch.properties
    +
    +###############################################################################
    +parseArgs() {
    +  case "$1" in
    +    QABUILD)
    +      ### Set QABUILD to true to indicate that this script is being run by Hudson
    +      QABUILD=true
    +      if [[ $# != 14 ]] ; then
    +        echo "ERROR: usage $0 QABUILD <PATCH_DIR> <PS_CMD> <WGET_CMD> <JIRACLI> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JIRA_PASSWD> <JAVA5_HOME> <CURL_CMD>"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$2
    +      PS=$3
    +      WGET=$4
    +      JIRACLI=$5
    +      GIT=$6
    +      GREP=$7
    +      PATCH=$8
    +      FINDBUGS_HOME=$9
    +      FORREST_HOME=${10}
    +      BASEDIR=${11}
    +      JIRA_PASSWD=${12}
    +      JAVA5_HOME=${13}
    +      CURL=${14}
    +      if [ ! -e "$PATCH_DIR" ] ; then
    +        mkdir -p $PATCH_DIR
    +      fi
    +
    +      ;;
    +    DEVELOPER)
    +      ### Set QABUILD to false to indicate that this script is being run by a developer
    +      QABUILD=false
    +      if [[ $# != 10 ]] ; then
    +        echo "ERROR: usage $0 DEVELOPER <PATCH_FILE> <SCRATCH_DIR> <GIT_CMD> <GREP_CMD> <PATCH_CMD> <FINDBUGS_HOME> <FORREST_HOME> <WORKSPACE_BASEDIR> <JAVA5_HOME>"
    +        cleanupAndExit 0
    +      fi
    +      ### PATCH_FILE contains the location of the patchfile
    +      PATCH_FILE=$2
    +      if [[ ! -e "$PATCH_FILE" ]] ; then
    +        echo "Unable to locate the patch file $PATCH_FILE"
    +        cleanupAndExit 0
    +      fi
    +      PATCH_DIR=$3
    +      ### Check if $PATCH_DIR exists. If it does not exist, create a new directory
    +      if [[ ! -e "$PATCH_DIR" ]] ; then
    +	mkdir "$PATCH_DIR"
    +	if [[ $? == 0 ]] ; then
    +	  echo "$PATCH_DIR has been created"
    +	else
    +	  echo "Unable to create $PATCH_DIR"
    +	  cleanupAndExit 0
    +	fi
    +      fi
    +      GIT=$4
    +      GREP=$5
    +      PATCH=$6
    +      FINDBUGS_HOME=$7
    +      FORREST_HOME=$8
    +      BASEDIR=$9
    +      JAVA5_HOME=${10}
    +      ### Obtain the patch filename to append it to the version number
    +      defect=`basename $PATCH_FILE`
    --- End diff --
    
    With git flow, I guess we don't care about the patch file name (e.g. ZOOKEEPER-1234.patch), so this could be removed?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper pull request #97: ZOOKEEPER-2624: Add test script for pull request...

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on a diff in the pull request:

    https://github.com/apache/zookeeper/pull/97#discussion_r86386008
  
    --- Diff: build.xml ---
    @@ -1622,6 +1623,26 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
         	</exec>
          </target>
     
    +     <target name="qa-test-pullrequest" depends="findbugs.check,forrest.check">
    +        <exec executable="bash" failonerror="true">
    +                <arg value="${test_pullrequest_sh}"/>
    +                <arg value="QABUILD"/>
    +                <arg value="${scratch.dir}"/>
    +                <arg value="${ps.cmd}"/>
    +                <arg value="${wget.cmd}"/>
    +                <arg value="${jiracli.cmd}"/>
    +                <arg value="${git.cmd}"/>
    +                <arg value="${grep.cmd}"/>
    +                <arg value="${patch.cmd}"/>
    +                <arg value="${findbugs.home}"/>
    +                <arg value="${forrest.home}"/>
    +                <arg value="${basedir}"/>
    +                <arg value="${jira.passwd}"/>
    +                <arg value="${java5.home}"/>
    +                <arg value="${curl.cmd}"/>
    --- End diff --
    
    yes, it was intentionally left out. the `defect` number is the jira number. in the jira qa, that is being set by the trigger of the jira patch. if you look further down in the script, you'll see that I'm setting the `defect` variable by extracting the number from the PR title. I expect the PR title to contain the jira title `ZOOKEEPER-XXXX: Bla bla bla`. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] zookeeper issue #97: ZOOKEEPER-2624: Add test script for pull requests

Posted by fpj <gi...@git.apache.org>.
Github user fpj commented on the issue:

    https://github.com/apache/zookeeper/pull/97
  
    @breed @rgs1 let's get this in, I'll create a jira to cleanup the script. Note that a lot of the issues you pointed out are legacy from the other script, so we will probably want to address in both.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---