Catch aplay error messages in apause.exp and exit #1226
Conversation
The script was originally tested with arecord but it works with aplay exactly the same. Stop saying "recording" when playing, it's confusing. Signed-off-by: Marc Herbert <[email protected]>
Fail fast and avoid timeouts. This is especially important considering pause will soon be disabled by default: - thesofproject/linux#5041 Signed-off-by: Marc Herbert <[email protected]>
Experience with thesofproject/linux#5109 shows that this warning never seems harmless: the test ends up timing out and failing anyway. So, better failing fast for clearer and better logs. Also increase the log level of press_space() to avoid state confusion. Signed-off-by: Marc Herbert <[email protected]>
|
Perfect demonstration of the value of this PR: Apparent memory corruption here: https://sof-ci.01.org/softestpr/PR1226/build710/devicetest/index.html?model=LNLM_SDW_AIOC&testcase=verify-kernel-boot-log is unrelated issue thesofproject/linux#5124 Everything else is 100% green. |
|
SOFCI TEST |
| # When multiple patterns match, first pattern wins. | ||
|
|
||
| -nocase -re {error.*\r|PAUSE.*no[[:blank:]]*hw[[:blank:]]*support.*\r} { |
There was a problem hiding this comment.
laughing at my desk at 8am, no idea what this does haha.
There was a problem hiding this comment.
It took me only a couple days of practice to become productive in Tcl and expect (without any "stackoverflow garbage in, AI garbage out!"); it's not so bad.
{ } is like the single quotes in shell except they can be nested.
The basic syntax of the expect instruction is just:
expect {
{ pattern1 } { action1 }
{ pattern2 } { action2 }
...
}
-nocase -re means the pattern is a case-insensitive regular expression (as opposed to globbing).
That regular expression has itself nothing specific to Tcl.
Thanks for taking a look!
| log 0 "ERROR: $buffer_with_lf" | ||
| exit 1 | ||
| } | ||
|
|
There was a problem hiding this comment.
just to be clear on the commit message: PAUSE will be disabled unless we have a kernel parameter that keeps it enabled. All CI platforms SHALL keep the pause enabled.
There was a problem hiding this comment.
That's what I tried to say in the commit message but more briefly and without getting into Intel specifics. Too briefly? I mean, should I change the commit message or is your comment here enough?
There was a problem hiding this comment.
thats' fine, just wanted to check we were aligned.
|
LNL https://sof-ci.01.org/softestpr/PR1226/build716/devicetest/index.html?model=LNLM_SDW_AIOC&testcase=multiple-pause-resume-50 has |
|
"pause push error: File descriptor in bad state" is not necessarily a problem, we see it all the time when resuming and INFO_RESUME is not supported. I've seen this log countless times and things were just fine, alsa-lib reinitialized whatever was needed with a prepare. Adding @ranj063 and @ujfalusi to make sure I am not completely mistaken. |
Mmmm.... questions:
|
Yes it's the same according to @ranj063 . Thanks for the reviews. This is a well tested and small change - easy to revert parts of it if needed so I'm merging it. |
Always better to "fail fast".
See commit messages.