-
Notifications
You must be signed in to change notification settings - Fork 45
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
Explicitly invoke start_test() in every test-case/*.sh #1187
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,12 @@ minvalue() { printf '%d' $(( "$1" < "$2" ? "$1" : "$2" )); } | |
# concurrency 1% of the time, detecting it 99% of the time is much | ||
# more than enough to spot reservation problems. | ||
# | ||
# 3. Register the func_exit_handler() EXIT trap that collects logs, kills | ||
# loggers and prints test results. | ||
# | ||
# 4. Waits for the FW to be successfully booted (can be disabled) | ||
# | ||
# | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: one blank line should be enough? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agree it's nitpick |
||
start_test() | ||
{ | ||
if is_subtest; then | ||
|
@@ -1018,4 +1024,3 @@ perf_analyze() | |
fi | ||
} | ||
|
||
start_test |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,8 @@ frequency=${OPT_VAL['F']} | |
sigmak=${OPT_VAL['k']} | ||
frames=${OPT_VAL['n']} | ||
|
||
start_test | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In commit message, what is the meaning of the opening sentence in relation to moving the start_test from lib.sh to test cases? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand the question... When
|
||
if [ "$pcm_p" = "" ]||[ "$pcm_c" = "" ]; | ||
then | ||
dloge "No playback or capture PCM is specified. Skip the alsabat test" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,8 @@ loop_cnt=${OPT_VAL['l']} | |
func_pipeline_export "$tplg" "eq:any" | ||
sofcard=${SOFCARD:-0} | ||
|
||
start_test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes this is a minor change: |
||
|
||
# Test equalizer | ||
func_test_eq() | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ loop_cnt=${OPT_VAL['l']} | |
out_dir=${OPT_VAL['o']} | ||
file_prefix=${OPT_VAL['f']} | ||
|
||
start_test | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All of this churn is to keep the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, the main benefit for sof_remove.sh is the ability to run it more than once. It used to fail when the firmware wasn't running. Also avoids some confusing logs: There will be a much bigger and much more important bug fix not related to sof_remove.sh, see commit message. Also restores the ability to source lib.sh interactively, also mentioned in the commit message There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The (non-mandatory) pulseaudio check is debatable. Including Anyway the most important and urgent benefit of this design fix is preparing the fix for #1036 and #1112. The same design fix just happens to help with some other scripts like sof_remove.sh. |
||
logger_disabled || func_lib_start_log_collect | ||
|
||
setup_kernel_check_point | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,11 @@ set -e | |
# shellcheck source=case-lib/lib.sh | ||
source "$(dirname "${BASH_SOURCE[0]}")"/../case-lib/lib.sh | ||
|
||
start_test | ||
# TODO: replace start_test with this: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This TODO will be next PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes immediately after this one. I want at least one run of daily tests between the two. |
||
trap 'print_test_result_exit $?' EXIT | ||
# https://github.com/thesofproject/sof-test/issues/1112 | ||
|
||
main() | ||
{ | ||
printf 'System booted at: '; uptime -s | ||
|
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.
How can it be disabled?
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.
NO_POLL_FW_LOADING
30 lines below, it's not that long of a function (as no function should be)Also documented in config.sh