Re: [Fuego] [PATCH] pam: add test case for pam.

[<Tim.Bird@sony.com>]

> -----Original Message-----
> From: Zheng Ruoqin
> 
> Signed-off-by: Zheng Ruoqin <zhengrq.fnst@cn.fujitsu.com>
> ---
>  engine/tests/Functional.pam/fuego_test.sh          | 19
> +++++++++++++++++++
>  engine/tests/Functional.pam/pam_test.sh            |  4 ++++
>  engine/tests/Functional.pam/parser.py              | 22
> ++++++++++++++++++++++
>  engine/tests/Functional.pam/spec.json              |  7 +++++++
>  .../Functional.pam/tests/pam_timestamp_check.sh    | 14 ++++++++++++++
>  5 files changed, 66 insertions(+)
>  create mode 100644 engine/tests/Functional.pam/fuego_test.sh
>  create mode 100644 engine/tests/Functional.pam/pam_test.sh
>  create mode 100644 engine/tests/Functional.pam/parser.py
>  create mode 100644 engine/tests/Functional.pam/spec.json
>  create mode 100644
> engine/tests/Functional.pam/tests/pam_timestamp_check.sh
> 
> diff --git a/engine/tests/Functional.pam/fuego_test.sh
> b/engine/tests/Functional.pam/fuego_test.sh
> new file mode 100644
> index 0000000..4be6015
> --- /dev/null
> +++ b/engine/tests/Functional.pam/fuego_test.sh
> @@ -0,0 +1,19 @@
> +function test_pre_check {
> +    is_on_target_path pam_timestamp_check PROGRAM_PAM
> +    assert_define PROGRAM_PAM "Missing 'pam_timestamp_check'
> program on target board"
> +}
> +
> +function test_deploy {
> +    put $TEST_HOME/pam_test.sh $BOARD_TESTDIR/fuego.$TESTDIR/
> +    put -r $TEST_HOME/tests $BOARD_TESTDIR/fuego.$TESTDIR/
> +}
> +
> +function test_run {
> +    report "cd $BOARD_TESTDIR/fuego.$TESTDIR;\
> +    sh -v pam_test.sh"
> +}
> +
> +function test_processing {
> +    log_compare "$TESTDIR" "1" "TEST-PASS" "p"
It is better to omit the line above.  If we expect 0 failures,
then it is better not to encode the number of successes.
Doing so makes it so we have to change this location as well
as the test, when we make a change to the number of testcases
in a particular test.

IOW - remove the line above.

> +    log_compare "$TESTDIR" "0" "TEST-FAIL" "n"
> +}
> diff --git a/engine/tests/Functional.pam/pam_test.sh
> b/engine/tests/Functional.pam/pam_test.sh
> new file mode 100644
> index 0000000..dd5ce37
> --- /dev/null
> +++ b/engine/tests/Functional.pam/pam_test.sh
> @@ -0,0 +1,4 @@
> +#!/bin/sh
> +for i in tests/*.sh; do
> +    sh $i
> +done
> diff --git a/engine/tests/Functional.pam/parser.py
> b/engine/tests/Functional.pam/parser.py
> new file mode 100644
> index 0000000..d85abd7
> --- /dev/null
> +++ b/engine/tests/Functional.pam/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')
> +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))

We do this often enough, that we should make a special case in the parser for
this exact sequence (maybe paramterized with a regular expression).

Something like:
#!/usr/bin/python
sys.path.insert(0, os.environ['FUEGO_CORE'] + '/engine/scripts/parser')
import sys
import common as plib
regex_string = '^ -> (.*): TEST-(.*)$'
sys.exit(plib.collect_PASS_FAIL_testcases(regex_string))

And, getting off on a tangent here, we should have the Fuego
core add $FUEGO_CORE/engine/scripts/parser to the
python path, before calling the parser module, and common.py
should be renamed plib.py, so this could be further reduced to:

#!/usr/bin/python
import sys, plib
sys.exit(plib.collect_PASS_FAIL_testcases(regex_string))

I don't suppose you would be willing to work on these
changes to the parsing core, would you?  That would be
really helpful, and would make future parsers much shorter.

> diff --git a/engine/tests/Functional.pam/spec.json
> b/engine/tests/Functional.pam/spec.json
> new file mode 100644
> index 0000000..64d84d7
> --- /dev/null
> +++ b/engine/tests/Functional.pam/spec.json
> @@ -0,0 +1,7 @@
> +{
> +    "testName": "Functional.pam",
> +    "specs": {
> +        "default": {}
> +    }
> +}
> +
> diff --git a/engine/tests/Functional.pam/tests/pam_timestamp_check.sh
> b/engine/tests/Functional.pam/tests/pam_timestamp_check.sh
> new file mode 100644
> index 0000000..785245f
> --- /dev/null
> +++ b/engine/tests/Functional.pam/tests/pam_timestamp_check.sh
> @@ -0,0 +1,14 @@
> +#!/bin/sh
> +
> +#  In target, run command pam_timestamp_check.
> +#  option "-k"
> +
> +test="pam_timestamp_check"
> +
> +if pam_timestamp_check -k
> +then
> +    echo " -> $test: TEST-PASS"
> +else
> +    echo " -> $test: TEST-FAIL"
> +fi;
> +
> --
> 1.8.3.1

I wish I knew what was being tested here.  Could you possibly
add a test.yaml for this test?  Even better would be if you
could create a file: Functional.pam.pam_timestamp_check_01.ftmp
to describe what this particular testcase is doing.

(See engine/tests/Functional.LTP.syscalls.inotify06.ftmp for an example
of a testcase ftmp file.)

Also, as with the others, I presume this is a placeholder for more pam
testcases to follow. 

I'm going to apply this one. 

Thanks,
 -- Tim