steveloughran commented on code in PR #3124:
URL: https://github.com/apache/hadoop/pull/3124#discussion_r989967669
##########
hadoop-tools/hadoop-azure/.gitignore:
##########
@@ -1,5 +1,6 @@
.checkstyle
bin/
-src/test/resources/combinationConfigFiles
src/test/resources/abfs-combination-test-configs.xml
dev-support/testlogs
+src/test/resources/accountSettings/*
+!src/test/resources/accountSettings/accountName_settings.xml.template
Review Comment:
can you add a newline
##########
hadoop-tools/hadoop-azure/dev-support/testrun-scripts/testsupport.sh:
##########
@@ -15,117 +15,88 @@
# See the License for the specific language governing permissions and
# limitations under the License.
-testresourcesdir=src/test/resources
-combconfsdir=$testresourcesdir/combinationConfigFiles
-combtestfile=$testresourcesdir/abfs-combination-test-configs.xml
+resourceDir=src/test/resources/
+accountSettingsFolderName=accountSettings
+combtestfile=$resourceDir
+combtestfile+=abfs-combination-test-configs.xml
+logdir=dev-support/testlogs/
-logdir=dev-support/testlogs
testresultsregex="Results:(\n|.)*?Tests run:"
-testresultsfilename=
-starttime=
-threadcount=
-defaultthreadcount=8
-
-properties=
-values=
-
-validate() {
- if [ -z "$threadcount" ] ; then
- threadcount=$defaultthreadcount
- fi
- numberegex='^[0-9]+$'
- if ! [[ $threadcount =~ $numberegex ]] ; then
- echo "Exiting. The script param (threadcount) should be a number"
- exit -1
- fi
- if [ -z "$combination" ]; then
- echo "Exiting. combination cannot be empty"
- exit -1
- fi
- propertiessize=${#properties[@]}
- valuessize=${#values[@]}
- if [ "$propertiessize" -lt 1 ] || [ "$valuessize" -lt 1 ] || [
"$propertiessize" -ne "$valuessize" ]; then
- echo "Exiting. Both properties and values arrays has to be populated and
of same size. Please check for combination $combination"
- exit -1
- fi
-
- for filename in "${combinations[@]}"; do
- if [[ ! -f "$combconfsdir/$filename.xml" ]]; then
- echo "Exiting. Combination config file ($combconfsdir/$combination.xml)
does not exist."
- exit -1
- fi
- done
-}
-
-checkdependencies() {
- if ! [ "$(command -v pcregrep)" ]; then
- echo "Exiting. pcregrep is required to run the script."
- exit -1
- fi
- if ! [ "$(command -v xmlstarlet)" ]; then
- echo "Exiting. xmlstarlet is required to run the script."
- exit -1
+accountConfigFileSuffix="_settings.xml"
+testOutputLogFolder=$logdir
+testlogfilename=combinationTestLogFile
+
+fullRunStartTime=$(date +%s)
+STARTTIME=$(date +%s)
+ENDTIME=$(date +%s)
+
+outputFormatOn="\033[0;95m"
+outputFormatOff="\033[0m"
+
+triggerRun()
+{
+ echo ' '
+ combination=$1
+ accountName=$2
+ runTest=$3
+ processcount=$4
+ cleanUpTestContainers=$5
+
+ if [ -z "$accountName" ]; then
+ logOutput "ERROR: Test account not configured. Re-run the script and
choose SET_OR_CHANGE_TEST_ACCOUNT to configure the test account."
+ exit 0;
Review Comment:
exit with an error code
##########
hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md:
##########
@@ -602,26 +602,64 @@ various test combinations, it will:
2. Run tests for all combinations
3. Summarize results across all the test combination runs.
-As a pre-requisite step, fill config values for test accounts and credentials
-needed for authentication in `src/test/resources/azure-auth-keys.xml.template`
-and rename as `src/test/resources/azure-auth-keys.xml`.
+Below are the pre-requiste steps to follow:
+1. Copy
-**To add a new test combination:** Templates for mandatory test combinations
-for PR validation are present in `dev-support/testrun-scripts/runtests.sh`.
-If a new one needs to be added, add a combination set within
-`dev-support/testrun-scripts/runtests.sh` similar to the ones already defined
-and
-1. Provide a new combination name
-2. Update properties and values array which need to be effective for the test
-combination
-3. Call generateconfigs
+ ./src/test/resources/azure-auth-keys.xml.template
+ TO
+ ./src/test/resources/azure-auth-keys.xml
+ Update account names that should be used in the test run for HNS and non-HNS
+ combinations in the 2 properties present in the xml (account name should be
+ without domain part), namely
+
+ fs.azure.hnsTestAccountName
+ fs.azure.nonHnsTestAccountName
+ azure-auth-keys.xml is listed in .gitignore, so any accidental account name
leak is prevented.
Review Comment:
add below
```
XInclude is supported, so for extra security secrets may be
kept out of the source tree then referenced through an an XInclude element:
<include xmlns="http://www.w3.org/2001/XInclude";
href="/users/self/.secrets/auth-keys.xml" />
```
As this is what I do everyone else should do the same
##########
hadoop-tools/hadoop-azure/dev-support/testrun-scripts/testsupport.sh:
##########
@@ -134,108 +105,68 @@ summary() {
echo "$combination"
echo "========================"
pcregrep -M "$testresultsregex" "$testlogfilename"
- } >> "$testresultsfilename"
+ } >> "$aggregatedTestResult"
printf "\n----- Test results -----\n"
pcregrep -M "$testresultsregex" "$testlogfilename"
-
secondstaken=$((ENDTIME - STARTTIME))
mins=$((secondstaken / 60))
secs=$((secondstaken % 60))
printf "\nTime taken: %s mins %s secs.\n" "$mins" "$secs"
- echo "Find test logs for the combination ($combination) in: $testlogfilename"
- echo "Find consolidated test results in: $testresultsfilename"
- echo "----------"
+ echo "Find test result for the combination ($combination) in:
$testlogfilename"
+ logOutput "Consolidated test result is saved in: $aggregatedTestResult"
+ echo "------------------------"
}
-init() {
- checkdependencies
- if ! mvn clean install -DskipTests
- then
- echo ""
- echo "Exiting. Build failed."
- exit -1
+checkdependencies() {
+ if ! [ "$(command -v pcregrep)" ]; then
+ logOutput "Exiting. pcregrep is required to run the script."
+ exit 1
fi
- starttime=$(date +"%Y-%m-%d_%H-%M-%S")
- mkdir -p "$logdir"
- testresultsfilename="$logdir/$starttime/Test-Results.txt"
- if [[ -z "$combinations" ]]; then
- combinations=( $( ls $combconfsdir/*.xml ))
+ if ! [ "$(command -v xmlstarlet)" ]; then
+ logOutput "Exiting. xmlstarlet is required to run the script."
+ exit 1
fi
}
-runtests() {
- parseoptions "$@"
- validate
- if [ -z "$starttime" ]; then
- init
- fi
- shopt -s nullglob
- for combconffile in "${combinations[@]}"; do
- STARTTIME=$(date +%s)
- combination=$(basename "$combconffile" .xml)
- mkdir -p "$logdir/$starttime"
- testlogfilename="$logdir/$starttime/Test-Logs-$combination.txt"
- printf "\nRunning the combination: %s..." "$combination"
- setactiveconf
- mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount=$threadcount
verify >> "$testlogfilename" || true
- ENDTIME=$(date +%s)
- summary
- done
+formatxml() {
+ xmlstarlet fo -s 2 "$1" > "$1.tmp"
+ mv "$1.tmp" "$1"
}
-begin() {
- cleancombinationconfigs
+changeconf() {
+ xmlstarlet ed -P -L -d "/configuration/property[name='$1']" "$combtestfile"
+ xmlstarlet ed -P -L -s /configuration -t elem -n propertyTMP -v "" -s
/configuration/propertyTMP -t elem -n name -v "$1" -r
/configuration/propertyTMP -v property "$combtestfile"
+ if ! xmlstarlet ed -P -L -s "/configuration/property[name='$1']" -t elem -n
value -v "$2" "$combtestfile"
+ then
+ logOutput "Exiting. Changing config property failed."
+ exit 1
+ fi
}
-parseoptions() {
-runactivate=0
-runtests=0
- while getopts ":c:a:t:" option; do
- case "${option}" in
- a)
- if [[ "$runactivate" -eq "1" ]]; then
- echo "-a Option is not multivalued"
- exit 1
- fi
- runactivate=1
- combination=$(basename "$OPTARG" .xml)
- ;;
- c)
- runtests=1
- combination=$(basename "$OPTARG" .xml)
- combinations+=("$combination")
- ;;
- t)
- threadcount=$OPTARG
- ;;
- *|?|h)
- if [[ -z "$combinations" ]]; then
- combinations=( $( ls $combconfsdir/*.xml ))
- fi
- combstr=""
- for combconffile in "${combinations[@]}"; do
- combname=$(basename "$combconffile" .xml)
- combstr="${combname}, ${combstr}"
- done
- combstr=${combstr:0:-2}
-
- echo "Usage: $0 [-n] [-a COMBINATION_NAME] [-c COMBINATION_NAME] [-t
THREAD_COUNT]"
- echo ""
- echo "Where:"
- echo " -a COMBINATION_NAME Specify the combination name which needs
to be activated."
- echo " Configured combinations: ${combstr}"
- echo " -c COMBINATION_NAME Specify the combination name for test
runs"
- echo " -t THREAD_COUNT Specify the thread count"
- exit 1
- ;;
- esac
- done
- if [[ "$runactivate" -eq "1" && "$runtests" -eq "1" ]]; then
- echo "Both activate (-a option) and test run combinations (-c option)
cannot be specified together"
+init() {
+ checkdependencies
+ if ! mvn clean install -DskipTests
+ then
+ echo ""
+ echo "Exiting. Build failed."
exit 1
fi
- if [[ "$runactivate" -eq "1" ]]; then
- setactiveconf
- exit 0
- fi
-}
+ starttime=$(date +"%Y-%m-%d_%H-%M-%S")
+ testOutputLogFolder+=$starttime
+ mkdir -p "$testOutputLogFolder"
+ aggregatedTestResult="$testOutputLogFolder/Test-Results.txt"
+ }
+
+ printAggregate() {
+ echo :::: AGGREGATED TEST RESULT ::::
+ cat "$aggregatedTestResult"
+ fullRunEndTime=$(date +%s)
+ fullRunTimeInSecs=$((fullRunEndTime - fullRunStartTime))
+ mins=$((fullRunTimeInSecs / 60))
+ secs=$((fullRunTimeInSecs % 60))
+ printf "\nTime taken: %s mins %s secs.\n" "$mins" "$secs"
+ }
+
+logOutput() {
+ echo -e "$outputFormatOn" "$1" "$outputFormatOff"
+}
Review Comment:
nit: add newline at EOF
##########
hadoop-tools/hadoop-azure/src/test/resources/azure-test.xml:
##########
@@ -70,4 +97,4 @@
<fallback />
</include>
-</configuration>
+</configuration>
Review Comment:
nit, add a newline before the EOF
--
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: common-issues-unsubscr...@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org
