-
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
Explicitly invoke start_test() in every test-case/*.sh #1187
Changes from 1 commit
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 |
---|---|---|
|
@@ -1024,4 +1024,3 @@ perf_analyze() | |
fi | ||
} | ||
|
||
start_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.
In commit message, what is the meaning of the opening sentence in relation to moving the start_test from lib.sh to test cases?
Stop unconditionally imposing start_test() to everything sourcing lib.sh
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.
I'm not sure I understand the question...
Now it's explicit, so each test has a choice to call
start_test
or not. That's all this PR is about.When
start_test
was inlib.sh
, tests had no choice but to runstart_test
start_test
started as a small thing so running it unconditionally was a "small bug" but it has kept growing.