adelapena commented on code in PR #1984:
URL: https://github.com/apache/cassandra/pull/1984#discussion_r1016796860
##########
.circleci/generate.sh:
##########
@@ -72,10 +80,12 @@ highres=false
env_vars=""
has_env_vars=false
check_env_vars=true
-while getopts "e:almhf" opt; do
+while getopts "e:almhfb:" opt; do
case $opt in
a ) all=true
;;
+ b ) BASE_BRANCH="$OPTARG"
+ ;;
Review Comment:
As before, I'd put this after -l/-m/-h
##########
.circleci/generate.sh:
##########
@@ -184,36 +194,43 @@ fi
# add new or modified tests to the sets of tests to be repeated
if (!($all)); then
- add_diff_tests ()
- {
- dir="${BASEDIR}/../${2}"
- diff=$(git --no-pager diff --name-only --diff-filter=AMR
${BASE_BRANCH}...HEAD ${dir})
- tests=$( echo "$diff" \
- | grep "Test\\.java" \
- | sed -e "s/\\.java//" \
- | sed -e "s,^${2},," \
- | tr '/' '.' \
- | grep ${3} )
- for test in $tests; do
- echo " $test"
- has_env_vars=true
- if echo "$env_vars" | grep -q "${1}="; then
- env_vars=$(echo "$env_vars" | sed -e "s/${1}=/${1}=${test},/")
- elif [ -z "$env_vars" ]; then
- env_vars="${1}=${test}"
- else
- env_vars="$env_vars|${1}=${test}"
- fi
- done
- }
+ # Sanity check that the referenced branch exists
+ if ! git show ${BASE_BRANCH} -- >&/dev/null; then
Review Comment:
This isn't POSIX compliant and doesn't work in, for example, Ubuntu's dash.
##########
.circleci/generate.sh:
##########
@@ -184,36 +194,43 @@ fi
# add new or modified tests to the sets of tests to be repeated
if (!($all)); then
- add_diff_tests ()
- {
- dir="${BASEDIR}/../${2}"
- diff=$(git --no-pager diff --name-only --diff-filter=AMR
${BASE_BRANCH}...HEAD ${dir})
- tests=$( echo "$diff" \
- | grep "Test\\.java" \
- | sed -e "s/\\.java//" \
- | sed -e "s,^${2},," \
- | tr '/' '.' \
- | grep ${3} )
- for test in $tests; do
- echo " $test"
- has_env_vars=true
- if echo "$env_vars" | grep -q "${1}="; then
- env_vars=$(echo "$env_vars" | sed -e "s/${1}=/${1}=${test},/")
- elif [ -z "$env_vars" ]; then
- env_vars="${1}=${test}"
- else
- env_vars="$env_vars|${1}=${test}"
- fi
- done
- }
+ # Sanity check that the referenced branch exists
+ if ! git show ${BASE_BRANCH} -- >&/dev/null; then
+ echo "Unknown base branch: ${BASE_BRANCH}. Skipping detection of changed
tests."
+ echo "If you wish to detect changed tests, please use the '-b' flag or set
the"
+ echo "BASE_BRANCH environment variable to an existing branch name (e.g.
origin/${BASE_BRANCH})."
Review Comment:
We could use the `die` function that immediately returns with exit code 1,
so we don't have to put the diff code in an `else` branch.
##########
.circleci/generate.sh:
##########
@@ -33,6 +37,10 @@ print_help()
echo " -a Generate the default config.yml using low resources and the
three templates"
echo " (config.yml.LOWRES, config.yml.MIDRES and config.yml.HIGHRES).
Use this for"
echo " permanent changes in config-2_1.yml that will be committed to
the main repo."
+ echo " -b Specify the base git branch for comparison when determining
changed tests to"
+ echo " repeat. You can also set the BASE_BRANCH environment variable to
control this."
+ echo " Defaults to ${BASE_BRANCH}. Note that this option is not used
when the '-a' flag"
+ echo " is specified."
Review Comment:
I'd put this below the -l/-m/-h flags, so we have the closely related
-a/-l/-m/-h together.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
