-
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
Add new Expect script tools/apause.exp
#1218
Conversation
stable-v2.2 results look good https://sof-ci.01.org/softestpr/PR1218/build598/devicetest/index.html LNL failures in https://sof-ci.01.org/softestpr/PR1218/build600/devicetest/index.html are unrelated |
NOCODEC timeouts in https://sof-ci.01.org/softestpr/PR1218/build597/devicetest/index.html :-( HDA is OK. Test just taking too long? More than 1h each time! Same in https://sof-ci.01.org/softestpr/PR1218/build599/devicetest/index.html: only NOCODEC+multiple is timing out after 1+h, everything else is OK Combinatorial explosion... already discussed in
|
With the loop count reduced to 1 by default (also submitted separately at #1219), everything now passes: https://sof-ci.01.org/softestpr/PR1218/build601/devicetest/index.html and https://sof-ci.01.org/softestpr/PR1218/build604/devicetest/index.html?model=LNLM_RVP_HDA&testcase=check-pause-resume-playback-100 failed but it looks like an actual bug, not an issue with the test. LNLM_SDW_AIOC failed because thesofproject/linux#5098 |
So we can easily find other tools. Signed-off-by: Marc Herbert <[email protected]>
https://sof-ci.01.org/softestpr/PR1218/build632/devicetest/index.html?model=LNLM_RVP_NOCODEC&testcase=multiple-pause-resume-50 ends in some infinite loop of overruns (which are not firmware errors?) but that seems like a real issue, not a problem with the test. Everything else is 100% green |
Good example of MAX saturation timeout that would be addressed by this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Can you fixup the one bogus signed-off-by line?
So we can invoke other tools easily. Signed-off-by: Marc Herbert <[email protected]>
This all started as an effort to solve test failures caused by "MAX" volume. See thesofproject#931 and others linked from there for more context. Another obvious issue was the duplication of `expect` code across two tests (check-pause-resume.sh and multiple-pause-resume.sh) After a bit of time actually "testing the tests" I realized the original author did not really understand `expect` or the problem. So I rewrote the entire `expect` part. The list of previous issues is a bit too long not to forget any but here are some: - The script could get "out of sync" with aplay and report that it was paused when it was actually resumed! And vice-versa. - De-duplication; obviously. - Moving to a separate script also solves the following problems: - Can be invoked, tested and debugged separately outside any shell script - Simplifies quoting - Unlocks editor features like syntax checking - Proper understanding and handling of newlines. - The expect script does not "sleep" anymore, which stops backpressuring and blocking the console output from aplay - with unknown side effects! - Adding `log_user 0` makes the test logs readable at last - Add decent logging for easier maintenance - Logging timestamps demonstrate that the entire aproach is too slow for pause/resume cycles shorter than ~200 ms. Default values won't be changed yet but at least the problem is now obvious. - Handle "MAX" volume! Not an error yet because it happens really across the board (MAX does usualy not happen long enough to timeout and _fail_ across the board) but the code is ready to upgrade the "MAX" warning to an ERROR with a one-line change. - Report EOF and timeouts differently. - Probably others. Signed-off-by: Marc Herbert <[email protected]>
See long commit message in previous commit adding apause.exp Signed-off-by: Marc Herbert <[email protected]>
See long commit message in previous commit adding apause.exp Signed-off-by: Marc Herbert <[email protected]>
This all started as an effort to solve test failures caused by "MAX"
volume. See #931 and others linked from there for more context.
Another obvious issue was the duplication of
expect
code across twotests (check-pause-resume.sh and multiple-pause-resume.sh)
After a bit of time actually "testing the tests" I realized the original
author did not really understand
expect
or the problem. So I rewrotethe entire
expect
part. The list of previous issues is a bit too longnot to forget any but here are some:
The script could get "out of sync" with aplay and report that it was
paused when it was actually resumed! And vice-versa.
De-duplication; obviously.
Moving to a separate script also solves the following problems:
Proper understanding and handling of newlines.
The expect script does not "sleep" anymore, which stops backpressuring
and blocking the console output from aplay - with unknown side effects!
Adding
log_user 0
makes the test logs readable at lastAdd decent logging for easier maintenance
Logging timestamps demonstrate that the entire aproach is too slow for
pause/resume cycles shorter than ~200 ms. Default values won't be
changed yet but at least the problem is now obvious.
Handle "MAX" volume! Not an error yet because it happens really across
the board (MAX does usualy not happen long enough to timeout and
fail across the board) but the code is ready to upgrade the "MAX"
warning to an ERROR with a one-line change.
Report EOF and timeouts differently.
Probably others.
cc: