On 28 July 2014 13:38, Lisa Nguyen <lisa.ngu...@linaro.org> wrote: > On 27 July 2014 23:16, Sanjay Singh Rawat <sanjay.ra...@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.ngu...@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, Power Management Working Group Linaro.org │ Open source software for ARM SoCs | Follow Linaro: Facebook | Twitter | Blog irc: lisatn | lisa.ngu...@linaro.org _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev
