Focus on returning the results of each test script rather than the results of each subtest. This will help to keep the number of pm-qa test results consistent across multiple boards regardless of number of frequencies, cores, etc.
Examples before the refactoring:
https://validation.linaro.org/dashboard/streams /anonymous/lisatn/bundles/331786fb33a49b060adccf51bb509d5f286422e7/
Examples after the refactoring:
https://validation.linaro.org/dashboard/streams/anonymous /lisatn/bundles/3451b80ed9ba8a813b109dac1c41b09f0445f819/
Questions and comments are highly encouraged as it's possible that the logic can be improved, or my explanation can be clearer. Also, once the final version of this patch is accepted and merged, then there will be a follow up patch to update the pwrmgmt test definition, so LAVA can record PM-QA results accurately.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org --- cpufreq/cpufreq_09.sh | 2 +- include/functions.sh | 22 +++++++++++++++++++--- include/thermal_functions.sh | 6 +++++- thermal/thermal_06.sh | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/cpufreq/cpufreq_09.sh b/cpufreq/cpufreq_09.sh index 7b65eec..62c953d 100755 --- a/cpufreq/cpufreq_09.sh +++ b/cpufreq/cpufreq_09.sh @@ -65,7 +65,7 @@ save_governors supported=$(cat $CPU_PATH/cpu0/cpufreq/scaling_available_governors | grep "powersave") if [ -z "$supported" ]; then log_skip "powersave not supported" - exit 0 + return 0 fi
trap "restore_governors; sigtrap" SIGHUP SIGINT SIGTERM diff --git a/include/functions.sh b/include/functions.sh index 417c725..a4625ad 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -32,14 +32,29 @@ INC=0 CPU= pass_count=0 fail_count=0 +skip_count=0 +test_script_status="true"
test_status_show() { - echo "-------- total = $(($pass_count + $fail_count))" - echo "-------- pass = $pass_count" # report failure only if it is there if [ $fail_count -ne 0 ] ; then - echo "-------- fail = $fail_count" + test_script_status="false" + elif [[ $skip_count -ne 0 && $fail_count -eq 0 ]] ; then + test_script_status="skip" + elif [[ $pass_count -ne 0 && $skip_count -ne 0 ]] ; then + test_script_status="pass" fi + + #print test script result + echo " " + if [[ $test_script_status == "true" ]]; then + echo $TEST_NAME: "pass" + elif [[ $test_script_status == "skip" ]]; then + echo $TEST_NAME: "skip" + else + echo $TEST_NAME: "fail" + fi + echo " " }
log_begin() { @@ -54,6 +69,7 @@ log_end() { log_skip() { log_begin "$@" log_end "skip" + skip_count=$(($skip_count + 1)) }
for_each_cpu() { diff --git a/include/thermal_functions.sh b/include/thermal_functions.sh index a51240b..a719487 100644 --- a/include/thermal_functions.sh +++ b/include/thermal_functions.sh @@ -44,11 +44,12 @@ check_valid_temp() {
if [ $temp_val -gt 0 ]; then log_end "Ok" + pass_count=$(($pass_count + 1)) return 0 fi
log_end "Err" - + fail_count=$(($fail_count + 1)) return 1 }
@@ -126,10 +127,12 @@ check_valid_binding() { log_begin "checking $descr" if [ $trip_point_val -ge $trip_point_max ]; then log_end "Err" + fail_count=$(($fail_count + 1)) return 1 fi
log_end "Ok" + pass_count=$(($pass_count + 1)) return 0 }
@@ -165,6 +168,7 @@ for_each_cooling_device() { devices=$(ls $THERMAL_PATH | grep "cooling_device['$MAX_CDEV']") if [ "$devices" == "" ]; then log_skip "no cooling devices" + skip_count=$(($skip_count + 1)) return 0 fi
diff --git a/thermal/thermal_06.sh b/thermal/thermal_06.sh index 92c987a..d8877f0 100755 --- a/thermal/thermal_06.sh +++ b/thermal/thermal_06.sh @@ -30,7 +30,7 @@ source ../include/thermal_functions.sh
if [ "$thermal_try_max" -eq 0 ]; then log_skip "test of trip points being crossed" - exit 0 + return 0 fi
TEST_LOOP=100
On Friday 25 July 2014 04:32 AM, Lisa Nguyen wrote:
Focus on returning the results of each test script rather than the results of each subtest. This will help to keep the number of pm-qa test results consistent across multiple boards regardless of number of frequencies, cores, etc.
Examples before the refactoring:
https://validation.linaro.org/dashboard/streams /anonymous/lisatn/bundles/331786fb33a49b060adccf51bb509d5f286422e7/
Examples after the refactoring:
https://validation.linaro.org/dashboard/streams/anonymous /lisatn/bundles/3451b80ed9ba8a813b109dac1c41b09f0445f819/
for cpu0 many tests are skipped (which i VALID), but this increases the skip_count and is reporting the whole test as skipped, e.g. cpuidle_03 case in the logs. I think we should use some other function in the cpu0 exception case.
Questions and comments are highly encouraged as it's possible that the logic can be improved, or my explanation can be clearer. Also, once the final version of this patch is accepted and merged, then there will be a follow up patch to update the pwrmgmt test definition, so LAVA can record PM-QA results accurately.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org
cpufreq/cpufreq_09.sh | 2 +- include/functions.sh | 22 +++++++++++++++++++--- include/thermal_functions.sh | 6 +++++- thermal/thermal_06.sh | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/cpufreq/cpufreq_09.sh b/cpufreq/cpufreq_09.sh index 7b65eec..62c953d 100755 --- a/cpufreq/cpufreq_09.sh +++ b/cpufreq/cpufreq_09.sh @@ -65,7 +65,7 @@ save_governors supported=$(cat $CPU_PATH/cpu0/cpufreq/scaling_available_governors | grep "powersave") if [ -z "$supported" ]; then log_skip "powersave not supported"
- exit 0
- return 0 fi
trap "restore_governors; sigtrap" SIGHUP SIGINT SIGTERM diff --git a/include/functions.sh b/include/functions.sh index 417c725..a4625ad 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -32,14 +32,29 @@ INC=0 CPU= pass_count=0 fail_count=0 +skip_count=0 +test_script_status="true" test_status_show() {
- echo "-------- total = $(($pass_count + $fail_count))"
- echo "-------- pass = $pass_count" # report failure only if it is there if [ $fail_count -ne 0 ] ; then
echo "-------- fail = $fail_count"
- test_script_status="false"
- elif [[ $skip_count -ne 0 && $fail_count -eq 0 ]] ; then
- test_script_status="skip"
- elif [[ $pass_count -ne 0 && $skip_count -ne 0 ]] ; then
- test_script_status="pass" fi
- #print test script result
- echo " "
- if [[ $test_script_status == "true" ]]; then
- echo $TEST_NAME: "pass"
- elif [[ $test_script_status == "skip" ]]; then
- echo $TEST_NAME: "skip"
- else
- echo $TEST_NAME: "fail"
- fi
- echo " " }
log_begin() { @@ -54,6 +69,7 @@ log_end() { log_skip() { log_begin "$@" log_end "skip"
- skip_count=$(($skip_count + 1)) }
for_each_cpu() { diff --git a/include/thermal_functions.sh b/include/thermal_functions.sh index a51240b..a719487 100644 --- a/include/thermal_functions.sh +++ b/include/thermal_functions.sh @@ -44,11 +44,12 @@ check_valid_temp() { if [ $temp_val -gt 0 ]; then log_end "Ok"
- pass_count=$(($pass_count + 1)) return 0 fi
log_end "Err"
- fail_count=$(($fail_count + 1)) return 1 }
@@ -126,10 +127,12 @@ check_valid_binding() { log_begin "checking $descr" if [ $trip_point_val -ge $trip_point_max ]; then log_end "Err"
- fail_count=$(($fail_count + 1)) return 1 fi
log_end "Ok"
- pass_count=$(($pass_count + 1))
it will be better stubbing pass/fail/skip count modifiction inside log_end() , then we don't have to operate on the count separately
return 0} @@ -165,6 +168,7 @@ for_each_cooling_device() { devices=$(ls $THERMAL_PATH | grep "cooling_device['$MAX_CDEV']") if [ "$devices" == "" ]; then log_skip "no cooling devices"
- skip_count=$(($skip_count + 1)) return 0 fi
diff --git a/thermal/thermal_06.sh b/thermal/thermal_06.sh index 92c987a..d8877f0 100755 --- a/thermal/thermal_06.sh +++ b/thermal/thermal_06.sh @@ -30,7 +30,7 @@ source ../include/thermal_functions.sh if [ "$thermal_try_max" -eq 0 ]; then log_skip "test of trip points being crossed"
- exit 0
- return 0 fi
TEST_LOOP=100
On 27 July 2014 23:16, Sanjay Singh Rawat sanjay.rawat@linaro.org wrote:
On Friday 25 July 2014 04:32 AM, Lisa Nguyen wrote:
Focus on returning the results of each test script rather than the results of each subtest. This will help to keep the number of pm-qa test results consistent across multiple boards regardless of number of frequencies, cores, etc.
Examples before the refactoring:
https://validation.linaro.org/dashboard/streams /anonymous/lisatn/bundles/331786fb33a49b060adccf51bb509d5f286422e7/
Examples after the refactoring:
https://validation.linaro.org/dashboard/streams/anonymous /lisatn/bundles/3451b80ed9ba8a813b109dac1c41b09f0445f819/
for cpu0 many tests are skipped (which i VALID), but this increases the skip_count and is reporting the whole test as skipped, e.g. cpuidle_03 case in the logs. I think we should use some other function in the cpu0 exception case.
I was under the impression that enabling cpu0 only applies for the cpuhotplug module if we pass in hotplug_allow_cpu0=1 to make check. I don't recall running cpuidle tests on cpu0 especially if the platform we're running pm-qa on is a single processor. Am I overlooking something?
Questions and comments are highly encouraged as it's possible that the logic can be improved, or my explanation can be clearer. Also, once the final version of this patch is accepted and merged, then there will be a follow up patch to update the pwrmgmt test definition, so LAVA can record PM-QA results accurately.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org
cpufreq/cpufreq_09.sh | 2 +- include/functions.sh | 22 +++++++++++++++++++--- include/thermal_functions.sh | 6 +++++- thermal/thermal_06.sh | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/cpufreq/cpufreq_09.sh b/cpufreq/cpufreq_09.sh index 7b65eec..62c953d 100755 --- a/cpufreq/cpufreq_09.sh +++ b/cpufreq/cpufreq_09.sh @@ -65,7 +65,7 @@ save_governors supported=$(cat $CPU_PATH/cpu0/cpufreq/scaling_available_governors | grep "powersave") if [ -z "$supported" ]; then log_skip "powersave not supported"
- exit 0
- return 0 fi trap "restore_governors; sigtrap" SIGHUP SIGINT SIGTERM
diff --git a/include/functions.sh b/include/functions.sh index 417c725..a4625ad 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -32,14 +32,29 @@ INC=0 CPU= pass_count=0 fail_count=0 +skip_count=0 +test_script_status="true" test_status_show() {
- echo "-------- total = $(($pass_count + $fail_count))"
- echo "-------- pass = $pass_count" # report failure only if it is there if [ $fail_count -ne 0 ] ; then
echo "-------- fail = $fail_count"
test_script_status="false"- elif [[ $skip_count -ne 0 && $fail_count -eq 0 ]] ; then
test_script_status="skip"- elif [[ $pass_count -ne 0 && $skip_count -ne 0 ]] ; then
test_script_status="pass" fi- #print test script result
- echo " "
- if [[ $test_script_status == "true" ]]; then
echo $TEST_NAME: "pass"- elif [[ $test_script_status == "skip" ]]; then
echo $TEST_NAME: "skip"- else
echo $TEST_NAME: "fail"- fi
- echo " " } log_begin() {
@@ -54,6 +69,7 @@ log_end() { log_skip() { log_begin "$@" log_end "skip"
- skip_count=$(($skip_count + 1)) } for_each_cpu() {
diff --git a/include/thermal_functions.sh b/include/thermal_functions.sh index a51240b..a719487 100644 --- a/include/thermal_functions.sh +++ b/include/thermal_functions.sh @@ -44,11 +44,12 @@ check_valid_temp() { if [ $temp_val -gt 0 ]; then log_end "Ok"
pass_count=$(($pass_count + 1)) return 0 fi log_end "Err"
- fail_count=$(($fail_count + 1)) return 1 } @@ -126,10 +127,12 @@ check_valid_binding() { log_begin "checking $descr" if [ $trip_point_val -ge $trip_point_max ]; then log_end "Err"
fail_count=$(($fail_count + 1)) return 1 fi log_end "Ok"- pass_count=$(($pass_count + 1))
it will be better stubbing pass/fail/skip count modifiction inside log_end() , then we don't have to operate on the count separately
Okay, I'll look into this. The only reason why I had separate counters for fail, skip, and pass because if a few subtests had a combination of skip, okay, and fail for one test script, then the final result should be fail.
Thanks for the feedback, Sanjay.
On 28 July 2014 13:38, Lisa Nguyen lisa.nguyen@linaro.org wrote:
On 27 July 2014 23:16, Sanjay Singh Rawat sanjay.rawat@linaro.org wrote:
On Friday 25 July 2014 04:32 AM, Lisa Nguyen wrote:
Focus on returning the results of each test script rather than the results of each subtest. This will help to keep the number of pm-qa test results consistent across multiple boards regardless of number of frequencies, cores, etc.
Examples before the refactoring:
https://validation.linaro.org/dashboard/streams /anonymous/lisatn/bundles/331786fb33a49b060adccf51bb509d5f286422e7/
Examples after the refactoring:
https://validation.linaro.org/dashboard/streams/anonymous /lisatn/bundles/3451b80ed9ba8a813b109dac1c41b09f0445f819/
for cpu0 many tests are skipped (which i VALID), but this increases the skip_count and is reporting the whole test as skipped, e.g. cpuidle_03 case in the logs. I think we should use some other function in the cpu0 exception case.
I was under the impression that enabling cpu0 only applies for the cpuhotplug module if we pass in hotplug_allow_cpu0=1 to make check. I don't recall running cpuidle tests on cpu0 especially if the platform we're running pm-qa on is a single processor. Am I overlooking something?
Questions and comments are highly encouraged as it's possible that the logic can be improved, or my explanation can be clearer. Also, once the final version of this patch is accepted and merged, then there will be a follow up patch to update the pwrmgmt test definition, so LAVA can record PM-QA results accurately.
Signed-off-by: Lisa Nguyen lisa.nguyen@linaro.org
cpufreq/cpufreq_09.sh | 2 +- include/functions.sh | 22 +++++++++++++++++++--- include/thermal_functions.sh | 6 +++++- thermal/thermal_06.sh | 2 +- 4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/cpufreq/cpufreq_09.sh b/cpufreq/cpufreq_09.sh index 7b65eec..62c953d 100755 --- a/cpufreq/cpufreq_09.sh +++ b/cpufreq/cpufreq_09.sh @@ -65,7 +65,7 @@ save_governors supported=$(cat $CPU_PATH/cpu0/cpufreq/scaling_available_governors | grep "powersave") if [ -z "$supported" ]; then log_skip "powersave not supported"
- exit 0
- return 0 fi trap "restore_governors; sigtrap" SIGHUP SIGINT SIGTERM
diff --git a/include/functions.sh b/include/functions.sh index 417c725..a4625ad 100644 --- a/include/functions.sh +++ b/include/functions.sh @@ -32,14 +32,29 @@ INC=0 CPU= pass_count=0 fail_count=0 +skip_count=0 +test_script_status="true" test_status_show() {
- echo "-------- total = $(($pass_count + $fail_count))"
- echo "-------- pass = $pass_count" # report failure only if it is there if [ $fail_count -ne 0 ] ; then
echo "-------- fail = $fail_count"
test_script_status="false"- elif [[ $skip_count -ne 0 && $fail_count -eq 0 ]] ; then
test_script_status="skip"- elif [[ $pass_count -ne 0 && $skip_count -ne 0 ]] ; then
test_script_status="pass" fi- #print test script result
- echo " "
- if [[ $test_script_status == "true" ]]; then
echo $TEST_NAME: "pass"- elif [[ $test_script_status == "skip" ]]; then
echo $TEST_NAME: "skip"- else
echo $TEST_NAME: "fail"- fi
- echo " " } log_begin() {
@@ -54,6 +69,7 @@ log_end() { log_skip() { log_begin "$@" log_end "skip"
- skip_count=$(($skip_count + 1)) } for_each_cpu() {
diff --git a/include/thermal_functions.sh b/include/thermal_functions.sh index a51240b..a719487 100644 --- a/include/thermal_functions.sh +++ b/include/thermal_functions.sh @@ -44,11 +44,12 @@ check_valid_temp() { if [ $temp_val -gt 0 ]; then log_end "Ok"
pass_count=$(($pass_count + 1)) return 0 fi log_end "Err"
- fail_count=$(($fail_count + 1)) return 1 } @@ -126,10 +127,12 @@ check_valid_binding() { log_begin "checking $descr" if [ $trip_point_val -ge $trip_point_max ]; then log_end "Err"
fail_count=$(($fail_count + 1)) return 1 fi log_end "Ok"- pass_count=$(($pass_count + 1))
it will be better stubbing pass/fail/skip count modifiction inside log_end() , then we don't have to operate on the count separately
Okay, I'll look into this. The only reason why I had separate counters for fail, skip, and pass because if a few subtests had a combination of skip, okay, and fail for one test script, then the final result should be fail.
Thanks for the feedback, Sanjay.
Quick update: I moved the counts inside the log_end() function. Now currently debugging thermal functions and figuring out why a few test scripts (not all, thankfully) give the wrong test results.
I'm aiming to get v2 out by end of day today for review.
-
Lisa Nguyen -
Sanjay Singh Rawat