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

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

[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"
+    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))
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

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

[Tim.Bird]

> -----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

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

[Zheng, Ruoqin]

Hi Tim:

Last time you said:

> 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.

My question is that if I rename common.py to plib.py,  it will affect other testcases or modules who have imported common.

If so, should I modify all the testcases or modules?

--------------------------------------------------
Zheng Ruoqin
Nanjing Fujitsu Nanda Software Tech. Co., Ltd.(FNST)
ADDR.: No.6 Wenzhu Road, Software Avenue,
       Nanjing, 210012, China
MAIL : zhengrq.fnst@cn.fujistu.com


> -----Original Message-----
> From: Tim.Bird@sony.com [mailto:Tim.Bird@sony.com]
> Sent: Wednesday, September 12, 2018 5:47 AM
> To: Zheng, Ruoqin/郑 若钦 <zhengrq.fnst@cn.fujitsu.com>;
> fuego@lists.linuxfoundation.org
> Subject: RE: [Fuego] [PATCH] pam: add test case for pam.
> 
> > -----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
> 
> 

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

[Tim.Bird]

> -----Original Message-----
> From: Zheng, Ruoqin 
> 
> Hi Tim:
> 
> Last time you said:
> 
> > 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.
> 
> My question is that if I rename common.py to plib.py,  it will affect other
> testcases or modules who have imported common.

Yes.  However, I was hoping that we could keep backwards compatibility
by having a symlink common.py to plib.py.  I haven't tested it, but I'm hoping 
it would work.

> 
> If so, should I modify all the testcases or modules?
That would be great, but if the symlink works, it wouldn't be a strong requirement.
I would like to change the import line for all parser modules that reference common.py

OK - I just looked, and there is already a python module called plib.  
See https://pypi.org/project/plib/

If we proceed with this change ,then it would be better to called the module something besides plib.
However, that ends up changing a lot more code, since all references to plib in the code, and not just
the import line, would have to change.  I think that is too big of a change, and I'd like to take the time to
figure out a good name that has no conflicts with other systems.

So, for now, just do the other parts I described.  But please do change other test's parser modules
to use the changes, where applicable. 

Thanks very much for working on this.
 -- Tim