Re: [Fuego] [PATCH v2] Add test case of service ipmi.

[<Tim.Bird@sony.com>]

OK - Thanks for this test.

Please understand that I'm not very fond of these tests that
only test generic features of a service (like 'can it load', 'does its
usage line work').  I would much prefer to get tests that actually
utilize or stress the functionality of the service under test.

Among the tests submitted last week, the brctrl test stands out
as meeting that criteria.

I'm still accepting this, but I almost didn't.  With so little of the actual
functionality of ipmi being tested, this test is quite shallow.  And having
this test in our suite gives the impression that there's more substance
here than there really is.

Please try to include more test of the service functionality where possible.

Please note my other comments below.

> -----Original Message-----
> From: Wang Mingyu
> 
> IPMI is a set of standardized specifications for hardware-based platform
> management
> systems making it possible to control and monitor servers centrally.
> This test set is used to check if process can be generated.
> 
> Signed-off-by: Wang Mingyu <wangmy@cn.fujitsu.com>
> ---
>  tests/Functional.ipmi/fuego_test.sh    | 18 ++++++++++++++++++
>  tests/Functional.ipmi/ipmi_test.sh     |  4 ++++
>  tests/Functional.ipmi/parser.py        | 22 ++++++++++++++++++++++
>  tests/Functional.ipmi/spec.json        |  7 +++++++
>  tests/Functional.ipmi/tests/ipmi_ps.sh | 26
> ++++++++++++++++++++++++++
>  5 files changed, 77 insertions(+)
>  create mode 100644 tests/Functional.ipmi/fuego_test.sh
>  create mode 100755 tests/Functional.ipmi/ipmi_test.sh
>  create mode 100644 tests/Functional.ipmi/parser.py
>  create mode 100644 tests/Functional.ipmi/spec.json
>  create mode 100644 tests/Functional.ipmi/tests/ipmi_ps.sh
> 
> diff --git a/tests/Functional.ipmi/fuego_test.sh
> b/tests/Functional.ipmi/fuego_test.sh
> new file mode 100644
> index 0000000..7dd9060
> --- /dev/null
> +++ b/tests/Functional.ipmi/fuego_test.sh
> @@ -0,0 +1,18 @@
> +function test_pre_check {
> +    assert_has_program openipmi-helper
> +}
> +
> +function test_deploy {
> +    put $TEST_HOME/ipmi_test.sh $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put $FUEGO_CORE/scripts/fuego_board_function_lib.sh
> $BOARD_TESTDIR/fuego.$TESTDIR
> +    put -r $TEST_HOME/tests $BOARD_TESTDIR/fuego.$TESTDIR/
> +}
> +
> +function test_run {
> +    report "cd $BOARD_TESTDIR/fuego.$TESTDIR;\
> +    ./ipmi_test.sh"
> +}
> +
> +function test_processing {
> +    log_compare "$TESTDIR" "0" "TEST-FAIL" "n"
> +}
> diff --git a/tests/Functional.ipmi/ipmi_test.sh
> b/tests/Functional.ipmi/ipmi_test.sh
> new file mode 100755
> index 0000000..dd5ce37
> --- /dev/null
> +++ b/tests/Functional.ipmi/ipmi_test.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +for i in tests/*.sh; do
> +    sh $i
> +done
> diff --git a/tests/Functional.ipmi/parser.py b/tests/Functional.ipmi/parser.py
> new file mode 100644
> index 0000000..d85abd7
> --- /dev/null
> +++ b/tests/Functional.ipmi/parser.py
> @@ -0,0 +1,22 @@
> +#!/usr/bin/python
> +# See common.py for description of command-line arguments
> +
> +import os, sys, collections
> +
> +sys.path.insert(0, os.environ['FUEGO_CORE'] + '/engine/scripts/parser')
This line is not needed anymore in the next branch, and I removed it.

> +import common as plib
> +
> +measurements = {}
> +measurements = collections.OrderedDict()
> +
> +regex_string = '^ -> (.*): TEST-(.*)$'
> +matches = plib.parse_log(regex_string)
> +
> +if matches:
> +    for m in matches:
> +        measurements['default.' + m[0]] = 'PASS' if m[1] == 'PASS' else 'FAIL'
> +
> +# split the output for each testcase
> +plib.split_output_per_testcase(regex_string, measurements)
> +
> +sys.exit(plib.process(measurements))
> diff --git a/tests/Functional.ipmi/spec.json b/tests/Functional.ipmi/spec.json
> new file mode 100644
> index 0000000..9dd7151
> --- /dev/null
> +++ b/tests/Functional.ipmi/spec.json
> @@ -0,0 +1,7 @@
> +{
> +    "testName": "Functional.ipmi",
> +    "specs": {
> +        "default": {}
> +    }
> +}
> +
> diff --git a/tests/Functional.ipmi/tests/ipmi_ps.sh
> b/tests/Functional.ipmi/tests/ipmi_ps.sh
> new file mode 100644
> index 0000000..721f505
> --- /dev/null
> +++ b/tests/Functional.ipmi/tests/ipmi_ps.sh
> @@ -0,0 +1,26 @@
> +#!/bin/sh
> +
> +#  In the target start ipmi, and confirm the process condition by command
> ps.
> +
> +test="ps"
> +
> +. ./fuego_board_function_lib.sh
> +
> +set_init_manager
> +
> +service_status=$(get_service_status ipmi)
> +
> +exec_service_on_target ipmi stop
> +
> +if exec_service_on_target ipmi start
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> start of ipmi failed."
> +    echo " -> $test: TEST-FAIL"
> +fi
> +
> +if [ "$service_status" = "inactive" ]
> +then
> +    exec_service_on_target ipmi stop
> +fi
> --
> 1.8.3.1

Thanks.  Applied to my 'next' branch.

 -- Tim