Run URM Tests through Debian Packages - #528
Conversation
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
Please update the commit message to a more clear to reflect with the current changes.
|
|
||
| if ! pkg_ensure_package_set urm; then | ||
| log_skip "$TESTNAME SKIP - required package set is not available: urm" | ||
| echo "$TESTNAME SKIP" >"$RES_FILE" |
There was a problem hiding this comment.
"RES_FILE" and "TESTNAME" have not been initialized when this failure path executes. they are assigned later at lines 55–58. If package recovery fails, the redirect targets an empty filename, no ".res" file is generated.
Move the complete package-recovery block below:
TESTNAME="userspace-resource-manager"
test_path="$(find_test_case_by_name "$TESTNAME")"
cd "$test_path" || exit 1
RES_FILE="./${TESTNAME}.res"
| # shellcheck disable=SC1091 | ||
| . "$TOOLS/lib_pkg_provider.sh" | ||
|
|
||
| if ! pkg_ensure_package_set urm; then |
There was a problem hiding this comment.
Every Debian/Ubuntu run may perform avoidable package-manager, network, and package-upgrade work, increasing runtime, external dependencies, and flakiness.
Recommended fix:
if ! pkg_ensure_required_package_set_present urm; then
log_skip "$TESTNAME SKIP - failed to ensure required package set: urm"
echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi
The shared helper explicitly documents that it avoids package-manager/network work when the required package set is already complete and verifies the package set again after recovery.
1b6b3ff to
5ccd9f4
Compare
| # shellcheck disable=SC1091 | ||
| . "$TOOLS/lib_pkg_provider.sh" | ||
|
|
||
| if ! pkg_ensure_required_package_set_present urm; then |
There was a problem hiding this comment.
"pkg_ensure_required_package_set_present" treats a missing package-set mapping as failure. On Yocto/RPM/OPKG, where this PR intentionally provides no "urm" mapping, the helper first fails verification, "pkg_ensure_package_set" performs its no-op, and the final verification fails again.
The runner then writes SKIP and exits instead of continuing with the packages already supplied by the image.
Recommended fix:
if [ -f "$TOOLS/lib_pkg_provider.sh" ]; then
# shellcheck disable=SC1091
. "$TOOLS/lib_pkg_provider.sh"
if pkg_lookup_package_set urm >/dev/null 2>&1; then
if ! pkg_ensure_required_package_set_present urm; then
log_skip "$TESTNAME SKIP - failed to ensure required package set: urm"
echo "$TESTNAME SKIP" >"$RES_FILE"
exit 0
fi
else
log_info "No URM package-set mapping for this OS/provider; using image-provided assets"
fi
fi
| # Qualcomm userspace-resource-manager package set. | ||
| # | ||
| # Used by tests that explicitly call: | ||
| # pkg_ensure_package_set urm |
There was a problem hiding this comment.
The comment documents direct use of "pkg_ensure_package_set" and "pkg_ensure_optional_package_set", but the current runner uses "pkg_ensure_required_package_set_present". Update this block to document the actual required-set caller and clarify that absent mappings are intentionally ignored by the runner
5ccd9f4 to
e94cb66
Compare
| shift | ||
| done | ||
|
|
||
| if pkg_lookup_package_set urm >/dev/null 2>&1; then |
There was a problem hiding this comment.
I have two points to get confirmation. Then we are good to merge.
- Package recovery runs before the RUN_MODE=list branch at line 345. Therefore ./run.sh --list can invoke apt, install packages and trigger package post-install actions.
Skip package recovery when RUN_MODE=list. Only ensure the package set after confirming that an approved test will actually execute.
- Guard with command -v pkg_lookup_package_set and command -v pkg_ensure_required_package_set_present, or make the library explicitly required on mapped distributions.
5b3fd2f to
786bd7a
Compare
| exit 0 | ||
| fi | ||
|
|
||
| # ---------- Ensure packages (deferred: skip when RUN_MODE=list) ---------- |
There was a problem hiding this comment.
Move this package-recovery block to immediately after CLI parsing, while retaining the RUN_MODE != list guard. It must execute before the service gate, configuration validation and preflight_bins. Continue using pkg_lookup_package_set and pkg_ensure_required_package_set_present
786bd7a to
93c7ca0
Compare
| # Only recover the package set once we know an approved test will actually | ||
| # execute. Guard with command -v so the calls are safe even when | ||
| # lib_pkg_provider.sh was not sourced or the OS/provider has no mapping. | ||
| if [ "$RUN_MODE" != "list" ]; then |
There was a problem hiding this comment.
Maybe my earlier comments didn't convey in right way.
Package recovery is gated only by RUN_MODE != list it does not confirm that the value supplied through --bin is approved. Consequently, ./run.sh --bin typo or a missing --bin value can invoke package verification, network access, installation and package post-install actions before the runner later skips the unapproved test.
Move approved_tests/is_approved before this block and validate ONE_BIN, or construct the approved run list before recovery. Package recovery should execute only for --all or a validated, approved --bin
93c7ca0 to
a7a9020
Compare
| # unapproved test. | ||
| # Guard with command -v so the calls are safe even when lib_pkg_provider.sh | ||
| # was not sourced or the OS/provider has no mapping. | ||
| _pkg_recovery_needed=0 |
There was a problem hiding this comment.
Please remove trailing underscore before var/functions names.
On platforms which use Debian builds, like Glymur, enable test case runs by installing the appropriate debian tests package for URM, "userspace-resource-manager-tests". This package installs the necessary binaries and configs needed for testing. The run.sh script already has instructions for running the tests, fetching and reporting the results in an appropriate manner. Signed-off-by: Kartik Nema <kartnema@qti.qualcomm.com>
a7a9020 to
2f50adf
Compare
Srikanth Muppandam (smuppand)
left a comment
Srikanth Muppandam (smuppand)
left a comment
There was a problem hiding this comment.
LGTM
e2dda42
into
qualcomm-linux:main
2 participants
On platforms which use Debian builds, like Glymur, enable test case runs by installing the appropriate testing package for URM: "userspace-resource-manager-tests". This package installs the necessary binaries and configs needed for testing.
The run.sh script already has instructions for running the tests, fetching and reporting the results in an appropriate manner.