Skip to content

Modify URM Test Runner #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
smuppand merged 1 commit into from
May 18, 2026
+21 −30
Merged

Modify URM Test Runner #459

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 21 additions & 30 deletions Runner/suites/Performance/userspace-resource-manager/run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,34 @@ test_path="$(find_test_case_by_name "$TESTNAME")"
cd "$test_path" || exit 1
RES_FILE="./${TESTNAME}.res"

log_info "=== Checking Dependencies ==="
if ! check_dependencies awk grep pgrep date printf; then
log_skip "$TESTNAME SKIP – base tools missing"
echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi

# ---------- Lock (avoid concurrent runs on same host) ----------
LOCKFILE="/tmp/${TESTNAME}.lock"
LOCKDIR="/tmp/${TESTNAME}.lockdir"

if command -v flock >/dev/null 2>&1; then
exec 9>"$LOCKFILE"
if ! flock -n 9; then
log_warn "Another ${TESTNAME} run is active; skipping"
log_info "Active URM-related processes:"
pgrep -af 'userspace-resource-manager|Urm(Component|Integration)Tests|run.sh' || true
Copy link
Copy Markdown
Contributor

@smuppand smuppand May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pgrep is used before the dependency check and is not listed in check_dependencies

Copy link
Copy Markdown
Contributor Author

@kartnema kartnema May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved check_dependencies above in the script, and added pgrep to the list

echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi
trap 'exec 9>&-' EXIT INT TERM
else
if ! mkdir "$LOCKFILE" 2>/dev/null; then
log_warn "Another ${TESTNAME} run is active; skipping"
if ! mkdir "$LOCKDIR" 2>/dev/null; then
log_warn "Another ${TESTNAME} run is active or stale fallback lockdir exists: $LOCKDIR"
echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi
trap 'rmdir "$LOCKFILE" 2>/dev/null || true' EXIT INT TERM
trap 'rmdir "$LOCKDIR" 2>/dev/null || true' EXIT INT TERM
fi

# ---------- Approved list (pinned whitelist) ----------
Expand Down Expand Up @@ -147,16 +159,6 @@ suite_requires_base_cfgs() {
done
return 1
}
parse_and_score_log() {
logf="$1"
if grep -Eiq '(^|[^a-z])fail(ed)?([^a-z]|$)|Assertion failed|Terminating Suite|Segmentation fault|Backtrace' "$logf"; then
return 1
fi
if grep -Eiq 'Run Successful|executed successfully|Ran Successfully' "$logf"; then
return 0
fi
return 2
}
per_suite_timeout() {
case "$1" in
UrmComponentTests)
Expand Down Expand Up @@ -185,11 +187,6 @@ run_cmd_maybe_timeout() {
log_info "----------------------------------------------------------------------"
log_info "------------------- Starting ${TESTNAME} Testcase ----------------------"
log_info "=== Test Initialization ==="
if ! check_dependencies awk grep date printf; then
log_skip "$TESTNAME SKIP – base tools missing"
echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi

# ---------- Logs ----------
TS="$(date +%Y%m%d-%H%M%S)"
Expand Down Expand Up @@ -398,13 +395,7 @@ run_one() {
run_cmd_maybe_timeout "$bin" >"$tlog" 2>&1
rc=$?

parse_and_score_log "$tlog"
score=$?
if [ $score -eq 2 ] && [ $rc -eq 0 ]; then
score=0
fi

case $score in
case $rc in
Copy link
Copy Markdown
Contributor

@smuppand smuppand May 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only return codes 0 and 1 are handled. Any other non-zero return code, such as:

124 from timeout

126 command found but not executable

127 command not found

signal-based exits

will not be explicitly handled. Depending on shell behavior, this can cause run_one() to return success or an unintended status, leading to wrong PASS/FAIL accounting.

Copy link
Copy Markdown
Contributor Author

@kartnema kartnema May 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URM test framework handles most of the exceptions internally (through exception handling) and returns an aggregate status code (https://github.com/qualcomm/userspace-resource-manager/blob/main/tests/Utils/URMTests.cpp#L123), which will always be 0 or 1. I wanted to map the script behavior more closely with that of the URM Test Framework.

Also, since we are checking whether the executable exists (and whether it is executable) earlier in the script already, hence those issues won't factor in later.

In any case, i'll add a default (wildcard) to the shell script switching logic as a fallaback.

Copy link
Copy Markdown
Contributor

@smuppand smuppand May 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URM test framework handles most of the exceptions internally (through exception handling) and returns an aggregate status code (https://github.com/qualcomm/userspace-resource-manager/blob/main/tests/Utils/URMTests.cpp#L123), which will always be 0 or 1. I wanted to map the script behavior more closely with that of the URM Test Framework.

Also, since we are checking whether the executable exists (and whether it is executable) earlier in the script already, hence those issues won't factor in later.

In any case, i'll add a default (wildcard) to the shell script switching logic as a fallaback.

Thanks, that makes sense. If the URM framework guarantees 0/1 for framework-handled test failures, relying on rc is fine and is cleaner than parsing logs.

I would still keep the wildcard fallback in the shell case statement, because failures outside the URM framework can still return other codes, for example timeout wrapper rc=124, shell execution failures, or signal exits. Treating those as FAIL is safer than leaving the case unmatched.

Copy link
Copy Markdown
Contributor Author

@kartnema kartnema May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added fallback (*)

Copy link
Copy Markdown
Contributor Author

@kartnema kartnema May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

0)
log_pass "[TEST] $name PASS"
echo "PASS" >"$tres"
Expand All @@ -417,11 +408,11 @@ run_one() {
echo "[FAIL] $name (rc=$rc)" >>"$LOGDIR/summary.txt"
return 1
;;
2)
log_skip "[TEST] $name SKIP"
echo "SKIP" >"$tres"
echo "[SKIP] $name (rc=$rc)" >>"$LOGDIR/summary.txt"
return 2
*)
log_fail "[TEST] $name UNKNOWN RC=$rc"
echo "FAIL" >"$tres"
echo "[FAIL] $name (unexpected rc=$rc)" >>"$LOGDIR/summary.txt"
return 1
;;
esac
}
Expand Down
Loading