-
Notifications
You must be signed in to change notification settings - Fork 46
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
[BUG] "Test Result: FAIL!" is missing when firmware path is not found from journalctl #979
Comments
Originally posted in #973 (comment) |
Thanks @miRoox for the complex analysis! First I want to make clear this situation should almost never happen. In #973 we made the decision to scan the logs to find the firmware file name so now we must make absolutely sure the firmware file name can be found in the logs by any means necessary. If it's not then maybe a TIMEOUT is not so bad - because it should almost never happen. Now if we really wanted to fix this then maybe something like the UNTESTED patch below would help. But I think I'd prefer to wait and completely re-organize logging in sof-test because at the moment it's a big and complicated mess and the patch below would not make things simpler. diff --git a/case-lib/hijack.sh b/case-lib/hijack.sh
index c1728b28c368..643d670f6cd8 100644
--- a/case-lib/hijack.sh
+++ b/case-lib/hijack.sh
@@ -101,7 +101,7 @@ function func_exit_handler()
fi
- if is_ipc4 && is_firmware_file_zephyr; then
+ if ! logger_disabled && is_ipc4 && is_firmware_file_zephyr; then
local mtraceBin; mtraceBin=mtrace-reader.py
dlogi "pkill -TERM -f $mtraceBin"
sudo pkill -TERM -f "$mtraceBin" || {
diff --git a/case-lib/lib.sh b/case-lib/lib.sh
index 2f6444253a81..4993e44310db 100644
--- a/case-lib/lib.sh
+++ b/case-lib/lib.sh
@@ -731,6 +731,22 @@ is_ipc4()
logger_disabled()
{
+ # Cache the result for performance and to solve the "recursivity"
+ # issue #979 where the func_exit_handler() is calling one more time
+ # a function that just died.
+ if test -n "${_sof_logging}"; then
+ if $_sof_logging; then
+ # horrible double-negation. This function should have been
+ # "logger_enabled()" but too late now.
+ return 1
+ else # 0 = true = logging is disabled
+ return 0
+ fi
+ fi
+
+ # Default to false in case we die here
+ _sof_logging=false
+
local ldcFile
# Some firmware/OS configurations do not support logging.
ldcFile=$(find_ldc_file) || {
@@ -738,7 +754,7 @@ logger_disabled()
return 0 # 0 is 'true'
}
- # Disable logging when available...
+ # Disable logging when the -s option is available
if [ ${OPT_VAL['s']} -eq 0 ]; then
return 0
fi
@@ -757,7 +773,8 @@ logger_disabled()
return 0
fi
- return 1
+ _sof_logging=true
+ return 1 # 1 = false = not disabled
}
print_module_params() |
Describe the bug
When firmware path is not found from journalctl on an IPC4 test, it will immediately exit without report "Test Result: FAIL!". So our CI will treat it as TIMEOUT.
To Reproduce
It's hard to reproduce, but we can mock the case like #978.
Expected behavior
Report "Test Result: FAIL!" at the end of the log.
Detail Info
Screenshots or console output
First occurrence:
Second occurrence (after TIMEOUT):
The text was updated successfully, but these errors were encountered: