Skip to content

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.

Sign up for GitHub

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

Closed
fredoh9 wants to merge 1 commit into from
Closed

check-kmod: set the kernel check point before starting a test #655

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion test-case/check-kmod-load-unload-after-playback.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ OPT_HAS_ARG['d']=1 OPT_VAL['d']=3
OPT_NAME['p']='pulseaudio' OPT_DESC['p']='disable pulseaudio on the test process'
OPT_HAS_ARG['p']=0 OPT_VAL['p']=1

func_opt_parse_option "$@"
fxunc_opt_parse_option "$@"
Copy link
Collaborator

@marc-hb marc-hb Apr 23, 2021

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.

setup_kernel_check_point
Copy link
Member

@plbossart plbossart Apr 22, 2021 •
edited by marc-hb
Loading

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?

Copy link
Collaborator

@marc-hb marc-hb Apr 22, 2021

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?

Copy link

@aiChaoSONG aiChaoSONG Apr 23, 2021 •
edited
Loading

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

Copy link

@aiChaoSONG aiChaoSONG Apr 23, 2021

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.


tplg=${OPT_VAL['t']}
loop_cnt=${OPT_VAL['l']}
pb_duration=${OPT_VAL['d']}
Expand Down