adoroszlai commented on a change in pull request #2514:
URL: https://github.com/apache/ozone/pull/2514#discussion_r686680597
##########
File path: hadoop-ozone/dist/src/main/compose/testlib.sh
##########
@@ -297,10 +297,19 @@ cleanup_docker_images() {
generate_report(){
local title="${1:-${COMPOSE_ENV_NAME}}"
local dir="${2:-${RESULT_DIR}}"
+ local xunitdir=""
+ if test "$#" -eq 3; then
+ xunitdir="${3}"
+ fi
Review comment:
```suggestion
local xunitdir="${3:-}"
```
##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -44,6 +49,11 @@ if [ "$OZONE_WITH_COVERAGE" ]; then
cp /tmp/jacoco-combined.exec "$SCRIPT_DIR"/result
fi
-generate_report "acceptance" "${ALL_RESULT_DIR}"
+if [ -z "$XUNIT_RESULT_DIR" ]
+then
+ generate_report "acceptance" "${ALL_RESULT_DIR}"
+else
+ generate_report "acceptance" "${ALL_RESULT_DIR}" "${XUNIT_RESULT_DIR}"
+fi
Review comment:
This can be simplified, since `generate_report` can handle empty
parameter:
```suggestion
generate_report "acceptance" "${ALL_RESULT_DIR}" "${XUNIT_RESULT_DIR}"
```
##########
File path: hadoop-ozone/dist/src/main/compose/testlib.sh
##########
@@ -297,10 +297,19 @@ cleanup_docker_images() {
generate_report(){
local title="${1:-${COMPOSE_ENV_NAME}}"
local dir="${2:-${RESULT_DIR}}"
+ local xunitdir=""
+ if test "$#" -eq 3; then
+ xunitdir="${3}"
+ fi
if command -v rebot > /dev/null 2>&1; then
#Generate the combined output and return with the right exit code (note:
robot = execute test, rebot = generate output)
- rebot --reporttitle "${title}" -N "${title}" -d "${dir}" "${dir}/*.xml"
+ if [ -z "${xunitdir}" ]
+ then
Review comment:
```suggestion
if [ -z "${xunitdir}" ]; then
```
##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -18,12 +18,17 @@
#
# Test executor to test all the compose/*/test.sh test scripts.
+# Optional path parameter is to generate and store result file in XUNIT format
as well.
Review comment:
```suggestion
```
##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -18,12 +18,17 @@
#
# Test executor to test all the compose/*/test.sh test scripts.
+# Optional path parameter is to generate and store result file in XUNIT format
as well.
#
SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
ALL_RESULT_DIR="$SCRIPT_DIR/result"
PROJECT_DIR="$SCRIPT_DIR/.."
mkdir -p "$ALL_RESULT_DIR"
rm "$ALL_RESULT_DIR"/* || true
+XUNIT_RESULT_DIR=""
+if test "$#" -eq 1; then
+ XUNIT_RESULT_DIR="${1}"
+fi
Review comment:
I think `test-all.sh` should simply use `XUNIT_RESULT_DIR` environment
variable if it exists, instead of taking it from positional parameters. This
would be more consistent with other optional items (e.g. `OZONE_WITH_COVERAGE`
is in this script, `OM_SERVICE_ID`, `OZONE_TEST_SELECTOR`, `RESULT_DIR`, etc.
in `testlib.sh`).
```suggestion
```
--
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]
