Re: [PATCH v3 2/2] testsuite: Use `pktest` to test pickles and std

[Jose E. Marchesi]

Hi Mohammad.

> 2020-12-17  Mohammad-Reza Nabipoor  <m.nabipoor@yahoo.com>
>
>       * testsuite/lib/poke-pk.exp: New file.
>       * testsuite/poke.pickles/pickles.exp: Likewise.
>       * testsuite/poke.pickles/argp-test.pk: New unit test.
>       * testsuite/poke.pickles/color-test.pk: Likewise.
>       * testsuite/poke.pickles/id3v1-test.pk: Likewise.
>       * testsuite/poke.pickles/rgb24-test.pk: Likewise.
>       * testsuite/poke.std/std-test.pk: Likewise.
>       * testsuite/poke.std/std.exp: Re-write to use `poke-pk.exp`.
>       * testsuite/poke.argp/: Remove test directory.
>       * testsuite/poke.color/: Likewise.
>       * testsuite/poke.id3v1/: Likewise.
>       * testsuite/poke.rgb24/: Likewise.
>       * testsuite/Makefile.am (EXTRA_DIST): Add new tests, remove old ones.
> ---
>
> Hi, Jose!
>
> I removed the shell scripts that run the pktests. Now a new tcl proc
> translates the TAP to DejaGNU.
> Everything is much simpler now.

This looks great!

Just a little comment:

> +proc pk-runtest {pkfiles} {
> +    foreach pkfile $pkfiles {
> +        if { [tap_execute $pkfile] ne "" } {
> +            return 0
> +        }
> +    }
> +
> +    return 1
> +}

> +if {![pk-runtest [lsort [glob -nocomplain $srcdir/poke.pickles/*-test.pk]]]} 
> {
> +    fail "testing pickles failed"
> +}

If you put the fail in pk-runtest, then you don't need to repeat the
conditional in every file using it.  You can then write just:

pk-runtest [lsort ...]

Then pk-runtest can be simplified to:

proc pk-runtest {pkfiles} {
    foreach pkfile $pkfiles {
        if { [tap_execute $pkfile] ne "" } {
            fail
        }
    }
}