Skip to content

Enhanced watchdog test #120

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.

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

vnarapar
Copy link
Contributor

This commit is to enhance the watchdog test and to skip for QCS6490

Added soc_id function in functestlib.sh

@vnarapar vnarapar requested a review from smuppand July 15, 2025 09:14

getsocId() {
if [ -r /sys/devices/soc0/soc_id ]; then
cat /sys/devices/soc0/soc_id
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid an extra cat by using shell buit-ins.
Check for alternative pathsm since some kernels symlink differently.
Validate that the contents are non-empty and numeric

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the test to use shell builtins and check for alternative pathsm
Added check for contents to be numeric and nonempty

@@ -39,15 +39,26 @@ log_info "----------------------------------------------------------------------
log_info "-------------------Starting $TESTNAME Testcase----------------------------"
log_info "=== Test Initialization ==="

soc_id=$(getsocId)

if [ $soc_id = 498 ]; then
Copy link
Contributor

Choose a reason for hiding this comment

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

always quote variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added quotes

if [ -e /dev/watchdog ]; then
log_pass "/dev/watchdog node is present."
CONFIGS="CONFIG_WATCHDOG CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED"
check_kernel_config "$CONFIGS" || {
log_fail "$TESTNAME : Required kernel configs missing"
Copy link
Contributor

Choose a reason for hiding this comment

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

use log_pass/log_fail one per outcome, don't duplicate "Fail" in the message itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

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.

Few changes are needed.

@vnarapar
Copy link
Contributor Author

Updated comments

fi
done

log_error "soc_id file not found or not readable" >&2
Copy link
Contributor

Choose a reason for hiding this comment

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

Would like to print the permissions if it is not readable. Easy for debugging in CI environmnet. Else just rephrase to "$soc_id" path not found.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

CONFIGS="CONFIG_WATCHDOG CONFIG_WATCHDOG_HANDLE_BOOT_ENABLED"
for cfg in $CONFIGS; do
if ! check_kernel_config "$cfg" 2>/dev/null; then
log_fail "$cfg is not enabled"
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to log a message when checking the config, especially if it fails at that step. This will make it easier to debug failures in the CI environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Example logs of config failure
sh-5.1# ./run-test.sh watchdog
[Executing test case: watchdog] 2022-04-29 00:07:32 -
[INFO] 2022-04-29 00:07:32 - -----------------------------------------------------------------------------------------
[INFO] 2022-04-29 00:07:32 - -------------------Starting watchdog Testcase----------------------------
[INFO] 2022-04-29 00:07:32 - === Test Initialization ===
[INFO] 2022-04-29 00:07:33 - SOC ID is: 534
[PASS] 2022-04-29 00:07:33 - /dev/watchdog node is present.
[INFO] 2022-04-29 00:07:33 - Checking if CONFIG_WATCHDOG is enabled
[PASS] 2022-04-29 00:07:33 - Kernel config CONFIG_WATCHDOG is enabled
[INFO] 2022-04-29 00:07:33 - Checking if CONFIG_WATTTSAPP is enabled
[FAIL] 2022-04-29 00:07:33 - Kernel config CONFIG_WATTTSAPP is missing or not enabled
[FAIL] 2022-04-29 00:07:33 - CONFIG_WATTTSAPP is not enabled
[FAIL] 2022-04-29 00:07:33 - watchdog failed

[INFO] 2022-04-29 00:07:33 - ========== Test Summary ==========
PASSED:
None

FAILED:
watchdog

SKIPPED:
None
[INFO] 2022-04-29 00:07:33 - ==================================

Copy link
Contributor

Choose a reason for hiding this comment

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

In addition to dev node and kernel configuration checks, it would be beneficial to include some functional validation based on the type of build—whether it's a full build or a kernel-only build. For kernel builds, try opening and petting the watchdog, and for systemd-based builds, consider checking the system services.

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.

A few minor changes are needed; otherwise, it looks good to me.

This commit is to enhance the watchdog test and to skip for QCS6490

Added soc_id function in functestlib.sh

Signed-off-by: Vamsee Narapareddi <[email protected]>
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