Repository navigation
Stop ignoring various logger failures - #373
Conversation
xiulipan
left a comment
xiulipan
left a comment
There was a problem hiding this comment.
Do we need any check if logger is not running before exiting?
This looks wired if test pass but logger failed and whole test case will exit with failure.
| # logfile is set in a different file: lib.sh | ||
| # shellcheck disable=SC2154 | ||
| wcLog=$(wc -l "$logfile") | ||
| dlogi "nlines=$wcLog" |
There was a problem hiding this comment.
maybe a more readable log here? This looks like a cmd.
There was a problem hiding this comment.
In the next PR I will implement a sanity check on the number of lines. For now this is not great looking but it's readable and I'd rather not write and test code that will change in the next PR.
There was a problem hiding this comment.
Thanks @xiulipan !
Do we need any check if logger is not running before exiting? This looks wired if test pass but logger failed and whole test case will exit with failure.
I don't understand this comment sorry. We already had for a few weeks the "sof-logger was already dead" check that makes the test fail if the logger crashed. It works; I tested it before and after this PR.
| # logfile is set in a different file: lib.sh | ||
| # shellcheck disable=SC2154 | ||
| wcLog=$(wc -l "$logfile") | ||
| dlogi "nlines=$wcLog" |
There was a problem hiding this comment.
In the next PR I will implement a sanity check on the number of lines. For now this is not great looking but it's readable and I'd rather not write and test code that will change in the next PR.
3d6bf81 to
2af4f11
Compare
Incomplete fix for thesofproject#298: helps only when logger is not found or not executable. Print a warning and abort tests if they requested logging _and_ they use set -e _and_ they don't explicitly ignore the error. Also get rid of $loggerBin variable that made the logic more complicated, now use $SOFLOGGER only. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Likely to avoid the shell printing a "Killed logger" message, lib.sh was starting the logger with a "2>/dev/null" redirection. Here's a sample list of logger errors that have been discarded by this redirection since forever: error: Unable to open ldc file /etc/sof/sof-apl.ldc error: Unable to open in file /sys/kernel/debug/sof/trace error: Unable to open version file /sys/kernel/debug/sof/fw_version Remove this 2>/dev/null redirection which was an insane price to pay for making the logs "beautiful". While not important, let's make the logs "beautiful" anyway because we can: thanks to -SIGINT instead of SIGKILL. SIGKILL (-9) does not give a any chance for the logger to exit cleanly, flus outputs etc. and is only ever used as a last resort, see "man systemd.kill" etc. Also log the logger command complete with parameters and the size of its output at the end. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
2af4f11 to
0e78de4
Compare
|
@marc-hb Eve- though we had such failures in past, I want to start a discussion here: it that behavior right? Do we need to fail the case if we could not get the logger? As we have standalone test case for logger. |
The name of issue #297 is "Check for a valid trace for ALL tests" so I moved your question there. |
yes we want to fail if the logger is gone during a test. There could be something with a specific test that gets in the way of the DMA trace, and we want to trap this error. |
marc-hb
added
False Pass / green failure
area:logs
2 commits, see commit messages.
Green failures episode "the logger"
Incomplete fix for #297 , more coming.