-
Notifications
You must be signed in to change notification settings - Fork 59
check-kmod: set the kernel check point before starting a test #655
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
Conversation
In error case, no kernel check point causes TIMEOUT in SOF CI. The below line will be last log without verdict, then ends with TIMEOUT. [ERROR] Kernel check point "KERNEL_CHECKPOINT" is not properly set Signed-off-by: Fred Oh <fred.oh@linux.intel.com>
|
I was able to test with TGLU_RVP_NOCODEC. With this PR, there is verdict at the end. Without this PR, last error was And TIMEOUT errors can be found in internal daily test 3430 with TGLU_RVP_NOCODEC also. |
|
|
||
| func_opt_parse_option "$@" | ||
| fxunc_opt_parse_option "$@" | ||
| setup_kernel_check_point |
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.
@fredoh9 @marc-hb maybe a stupid question, but there was a similar change (EDIT: c76cb0b) from @aiChaoSONG for pm_runtime tests yesterday, so should we add this in ALL tests?
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.
This is more or less what I asked in #639 (comment)
Some (future?) tests may want to scan the entire log since boot though. For instance we discussed one "empty" test only for the purpose of checking SOF modules loaded at boot time. It's still not clear what such a test should do wrt checkpoints, @aiChaoSONG
After excluding aliases and non-audio tests I found only 2 tests that don't invoke setup_kernel_check_point() directly:
+test-case/check-audio-equalizer.sh
+test-case/check-volume-levels.sh
Would these two tests also timeout if they fail?
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.
@plbossart @marc-hb Previously, when we were using DMESG_KERNEL_LAST_LINE which acted like kernel check point, this variable is implicitly set up in lib.sh. But I think it is better to set up check point explicitly in every test case, this will tell where the log collect is started.
In PR #653, I add an option to disable check point, with it, we are able to collect log from kernel boot
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.
so should we add this in ALL tests
I have a round of clean up on this within the journalctl PR, only minor number of test case(s) which is not run in PR test or daily test may need this. And also, when this happens now, some boundary condition is triggered.
|
@fredoh9 , @aiChaoSONG can you say more about this TIMEOUT, why does it happen, where and how long it is? Can we please change this checkpoint failure so it fails immediately? EDIT: done in separate #661 |
|
In https://sof-ci.01.org/softestpr/PR655/build666/devicetest and https://sof-ci.01.org/softestpr/PR655/build687/devicetest/ the modified test failed on all platforms with: It's very good that it failed, no green failure for a change. It's not so good that it timed-out. |
Already answered 3 days ago by @aiChaoSONG in #653, sorry I've been running behind with code reviews. |
All checkpoint timeouts fixed in new #661 "don't die in the die handler" |
| OPT_HAS_ARG['p']=0 OPT_VAL['p']=1 | ||
|
|
||
| func_opt_parse_option "$@" | ||
| fxunc_opt_parse_option "$@" |
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.
While fixing this typo, can you also add set -e if testing is OK?
If set -e requires some other changes then forget it, does not really belong to this PR.
|
#661 is merged |
|
I'm confused @fredoh9 , @aiChaoSONG : don't we still want this? |
I think we still need this, we don't die but only print error messages, without this, we may see the error messages printed out. |
|
SOFCI TEST EDIT: the |
|
New KERNEL_CHECKPOINT discussion in |
In error case, no kernel check point causes TIMEOUT in SOF CI. The below
line will be last log without verdict, then ends with TIMEOUT.
[ERROR] Kernel check point "KERNEL_CHECKPOINT" is not properly set
Signed-off-by: Fred Oh fred.oh@linux.intel.com