smengcl commented on code in PR #11515:
URL: https://github.com/apache/ozone/pull/11515#discussion_r4267641845
##########
.github/workflows/intermittent-test-check.yml:
##########
@@ -213,6 +213,7 @@ jobs:
hadoop-ozone/dev-support/checks/junit.sh $args
-Dtest="$TEST_CLASS#$TEST_METHOD,Abstract*Test*\$*"
fi
env:
+ CHECK: flaky
Review Comment:
This changes the report directory to `target/flaky`. Shall we preserve the
existing output directory? Failure summary and artifact upload steps still read
`target/unit`
```suggestion
CHECK: flaky
OUTPUT_DIR: target/unit
```
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -55,10 +55,22 @@ if [[ -f
hadoop-ozone/dist/src/shell/ozone/ozone-functions.sh ]]; then
ozone_java_setup
fi
-mvn ${MAVEN_OPTIONS} clean
+CLEAN="clean"
-if [[ ${ITERATIONS} -gt 1 ]] && [[ ${OZONE_REPO_CACHED} == "false" ]]; then
- mvn ${MAVEN_OPTIONS} -DskipTests install
+if [[ ${ITERATIONS} -gt 1 ]]; then
+ CHECK=flaky
+
+ if [[ ${OZONE_REPO_CACHED} == "false" ]]; then
+ # Need to clean and build for local run.
+ # In flaky-test-check the jars come from separate workflow job.
+ mvn ${MAVEN_OPTIONS} -DskipTests "$@" clean install
+ CLEAN=""
+ fi
+fi
+
+if [[ ${CHECK} == "flaky" ]]; then
Review Comment:
Shall we preserve `CHECK` here? `_mvn_unit_report.sh` uses it for the
integration leak and cluster startup checks. Setting it to `flaky` skips those
checks for repeated integration runs, CMIIW.
```suggestion
if [[ ${OZONE_REPO_CACHED} == "false" ]]; then
# Need to clean and build for local run.
# In flaky-test-check the jars come from separate workflow job.
mvn ${MAVEN_OPTIONS} -DskipTests "$@" clean install
CLEAN=""
fi
fi
if [[ ${ITERATIONS} -gt 1 || ${CHECK} == "flaky" ]]; then
```
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -55,10 +55,22 @@ if [[ -f
hadoop-ozone/dist/src/shell/ozone/ozone-functions.sh ]]; then
ozone_java_setup
fi
-mvn ${MAVEN_OPTIONS} clean
+CLEAN="clean"
-if [[ ${ITERATIONS} -gt 1 ]] && [[ ${OZONE_REPO_CACHED} == "false" ]]; then
- mvn ${MAVEN_OPTIONS} -DskipTests install
+if [[ ${ITERATIONS} -gt 1 ]]; then
+ CHECK=flaky
+
+ if [[ ${OZONE_REPO_CACHED} == "false" ]]; then
+ # Need to clean and build for local run.
+ # In flaky-test-check the jars come from separate workflow job.
+ mvn ${MAVEN_OPTIONS} -DskipTests "$@" clean install
+ CLEAN=""
+ fi
+fi
+
+if [[ ${CHECK} == "flaky" ]]; then
+ # repeated runs must execute the test, not load it from the build cache
+ MAVEN_OPTIONS="${MAVEN_OPTIONS} -Ddevelocity.cache.remote.enabled=false"
Review Comment:
The local build cache is enabled outside GitHub Actions. Should repeated
runs also pass `-Ddevelocity.cache.local.enabled=false` to make sure tests are
not restored from the cache?
```suggestion
MAVEN_OPTIONS="${MAVEN_OPTIONS} -Ddevelocity.cache.remote.enabled=false
-Ddevelocity.cache.local.enabled=false"
```
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -75,13 +87,15 @@ for i in $(seq 1 ${ITERATIONS}); do
if [[ ${ITERATIONS} -gt 1 ]]; then
original_report_dir="${REPORT_DIR}"
REPORT_DIR="${original_report_dir}/iteration${i}"
- mkdir -p "${REPORT_DIR}"
fi
- mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" "$@"
"${PHASE}" \
- | tee "${REPORT_DIR}/output.log"
+ mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" "$@"
${CLEAN} "${PHASE}" \
+ | tee output.log
irc=$?
Review Comment:
IMO we should clear `CLEAN` after the first invocation. With
`OZONE_REPO_CACHED=true` and `-am`, later iterations clean the root `target/`,
deleting earlier summary entries and saved failure reports.
```suggestion
irc=$?
CLEAN=""
```
##########
hadoop-ozone/dev-support/checks/junit.sh:
##########
@@ -75,13 +87,15 @@ for i in $(seq 1 ${ITERATIONS}); do
if [[ ${ITERATIONS} -gt 1 ]]; then
original_report_dir="${REPORT_DIR}"
REPORT_DIR="${original_report_dir}/iteration${i}"
- mkdir -p "${REPORT_DIR}"
fi
- mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" "$@"
"${PHASE}" \
- | tee "${REPORT_DIR}/output.log"
+ mvn ${MAVEN_OPTIONS}
-Dmaven-surefire-plugin.argLineAccessArgs="${OZONE_MODULE_ACCESS_ARGS}" "$@"
${CLEAN} "${PHASE}" \
Review Comment:
Shall we keep cleanup and report collection consistent? With `-pl`, `clean`
now covers only the selected reactor modules, but `_mvn_unit_report.sh` still
scans the whole tree. A stale failing `TEST-*.xml` from an unselected module
can therefore make a passing local run fail; the previous standalone clean
removed it.
One option is to restore whole-tree cleanup before creating the report
directory. The later `clean test` still allows cache writes. This adds one
Maven invocation.
```diff
--- a/hadoop-ozone/dev-support/checks/junit.sh
+++ b/hadoop-ozone/dev-support/checks/junit.sh
@@ -55,4 +55,6 @@
ozone_java_setup
fi
+
+mvn ${MAVEN_OPTIONS} clean
CLEAN="clean"
```
--
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]
