Skip to content

userspace-resource-manager: Modify test runner for URM#352

Merged
smuppand merged 1 commit intoqualcomm-linux:mainfrom
kartnema:main
Mar 26, 2026
Merged

userspace-resource-manager: Modify test runner for URM#352
smuppand merged 1 commit intoqualcomm-linux:mainfrom
kartnema:main

Conversation

@kartnema
Copy link
Contributor

@kartnema kartnema commented Mar 17, 2026

Update the existing test runner for resource-tuner:

  • Changes to reflect project name change from resource-tuner to userspace-resource-manager.
  • Modify the list of test executables to be executed as part of automated testing.
  • Test Runner changes to use configs in /etc/urm/tests rather than /etc/urm/custom

@kartnema kartnema marked this pull request as draft March 17, 2026 06:15
@kartnema kartnema changed the title Modify test runner for URM userspace-resource-manager: Modify test runner for URM Mar 17, 2026
@kartnema kartnema marked this pull request as ready for review March 17, 2026 06:40
# base config requirement: accept common OR tests; skip only if BOTH missing
if suite_requires_base_cfgs "$name"; then
if [ $COMMON_OK -eq 0 ] && [ $CUSTOM_OK -eq 0 ]; then
if [ $COMMON_CONFIGS_OK -eq 0 ] && [ $TEST_CONFIGS_OK -eq 0 ] && [ $TEST_NODES_OK -eq 0 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this changes the gating semantics in a risky way. tests/nodes looks like a required input for the URM suites, but with this condition the suite will still run when configs are present and tests/nodes is missing. Should the nodes check remain a separate per-suite skip, instead of being folded into the “all three missing” condition?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the update test suites, both tests/nodes and tests/configs are required input, in addition to common configs.

The goal is since, running test case with tests/nodes present but tests/configs (or the other way around) does not make sense, hence a more exhaustive check needs to be added to see if all 3 requirements are satisfied. I'll make the code level checks, and change the comments as appropriate.

if suite_requires_base_cfgs "$name"; then
if [ $COMMON_OK -eq 0 ] && [ $CUSTOM_OK -eq 0 ]; then
if [ $COMMON_CONFIGS_OK -eq 0 ] && [ $TEST_CONFIGS_OK -eq 0 ] && [ $TEST_NODES_OK -eq 0 ]; then
log_skip "[CFG] Base configs missing (common/ AND custom/) — skipping $name"
Copy link
Contributor

Choose a reason for hiding this comment

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

This still says custom/, but the new path is tests/configs. Please update the message so runtime logs match the new layout.

@kartnema kartnema force-pushed the main branch 2 times, most recently from 0c9e298 to 3032b45 Compare March 24, 2026 09:02
@kartnema kartnema requested a review from smuppand March 24, 2026 09:02
#!/bin/sh
# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
# SPDX-License-Identifier: BSD-3-Clause# resource-tuner test runner (pinned whitelist)
# SPDX-License-Identifier: BSD-3-Clause# userspace-resource-manager test runner (pinned whitelist)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please split this into two separate comment lines so the SPDX tag stays valid and parseable.

SUITES_REQUIRE_BASE_CFGS="ResourceProcessorTests SignalConfigProcessorTests SysConfigAPITests \
ExtFeaturesParsingTests TargetConfigProcessorTests InitConfigParsingTests \
ResourceParsingTests ExtensionIntfTests resource_tuner_tests"
# Suites that need base configs (accept either common/ OR tests/)
Copy link
Contributor

Choose a reason for hiding this comment

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

The current logic below requires all of common/, tests/configs, and tests/nodes. Please update the comment so it matches the actual behavior.

- Service INACTIVE => overall SKIP (end early)
- Base configs: suites require common/ OR custom/ (skip only if BOTH missing)
- resource_tuner_tests additionally needs tests/Configs/ResourceSysFsNodes
- Base configs: suites require common/ OR tests/ (skip only if BOTH missing)
Copy link
Contributor

Choose a reason for hiding this comment

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

print_usage() is still describing the old semantics
but the implementation now skips when any of common/, tests/configs, or tests/nodes is missing. Please update the usage text so runtime help matches the current logic.

- `InitConfig.yaml`, `PropertiesConfig.yaml`, `ResourcesConfig.yaml`, `SignalsConfig.yaml`

- `custom/` (required files):
- `tests/configs/` (required files):
Copy link
Contributor

Choose a reason for hiding this comment

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

The tests/configs/ required-file list is missing Baseline.yaml, but run.sh currently requires it through RT_REQUIRE_TEST_FILES. Please add Baseline.yaml here so the README matches the implementation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, also renamed RT_REQUIRE_TEST_FILES to URM_REQUIRE_TEST_FILES, to align with project naming

- `InitConfig.yaml`, `PropertiesConfig.yaml`, `ResourcesConfig.yaml`, `SignalsConfig.yaml`, `TargetConfig.yaml`, `ExtFeaturesConfig.yaml`

If **both** trees are missing required files/dirs, config‑parsing suites are **SKIP** only (neutral).
- `tests/nodes/` (required files):
Copy link
Contributor

Choose a reason for hiding this comment

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

the runner checks that tests/nodes/ is present and non-empty, not that it contains a specific required file list. Please reword this to something like “must exist and be non-empty” for clarity.

- `RT_REQUIRE_COMMON_FILES`, `RT_REQUIRE_CUSTOM_FILES`: *space‑separated* filenames that must exist in `common/` / `custom/` respectively to treat that tree as present.
- `SERVICE_NAME`: systemd unit to check (default: `urm.service`)
- `RT_CONFIG_DIR`: root of config tree (default: `/etc/urm`)
- `RT_REQUIRE_COMMON_FILES`, `RT_REQUIRE_CUSTOM_FILES`: *space‑separated* filenames that must exist in `common/` / `tests/` respectively to treat that tree as present.
Copy link
Contributor

Choose a reason for hiding this comment

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

The environment override names here are out of sync with the script:

  • README says RT_CONFIG_DIR
  • run.sh uses URM_CONFIG_DIR
  • README says RT_REQUIRE_CUSTOM_FILES
  • run.sh uses RT_REQUIRE_TEST_FILES

Please update this section so the documented variable names match the actual implementation
introvert.com

Copy link
Contributor

Choose a reason for hiding this comment

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

the script now reads URM_CONFIG_DIR. Please update the example accordingly

## Exit status

The script writes the overall result to `resource-tuner.res`. The **process exit code is 0** in all cases in the current version (soft gating). If you want hard CI gating via non‑zero exit on FAIL, that can be added easily on request.
The script writes the overall result to `userspace-resource-manager.res`. The **process exit code is 0** in all cases in the current version (soft gating). If you want hard CI gating via non‑zero exit on FAIL, that can be added easily on request.
Copy link
Contributor

Choose a reason for hiding this comment

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

Current run.sh exits 1 on overall FAIL. Please update the Exit status section to match the actual behavior.

@kartnema kartnema force-pushed the main branch 2 times, most recently from 2226fa5 to 638b174 Compare March 25, 2026 08:39
#!/bin/sh
# Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
# SPDX-License-Identifier: BSD-3-Clause# resource-tuner test runner (pinned whitelist)
# SPDX-License-Identifier: BSD-3-Clause#
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny clean up it should just be # SPDX-License-Identifier: BSD-3-Clause

Signed-off-by: Kartik Nema <kartnema@qti.qualcomm.com>
Copy link
Contributor

@smuppand smuppand left a comment

Choose a reason for hiding this comment

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

LGTM

@smuppand smuppand merged commit 1991e58 into qualcomm-linux:main Mar 26, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants