Re: [PATCH v2 1/2] pickles: Add new pickle `pktest.pk`

[Jose E. Marchesi]

Hi Mohammad.


> +type PkTestFn = (string) void;
> +type PkTest = struct
> +  {
> +    string name;
> +    string skip;  /* Skip reason. If non-empty, test will be skipped */
> +    string todo;  /* TODO reason. If non-empty, test will be marked as TODO 
> */
> +    PkTestFn func;
> +  };

Since skip and todo are exclusive, you can use Poke's data integrity to
enforce it:

type PkTest = struct
  {
    string name;
    string skip;
    string todo : todo == "" || (todo != "" && skip == "");
    PkTestFn func;
  };

To ease describing constraint like that, in recutils I introduced an
"implies" operator =>.

A => B translates into: !A || (A && B).

Example from a recfile:

%constraint: Op = "10" => #Op3

meaning `if the Op in a record is "10" then it should have an Op3 as
well'.

We may introduce this operator in poke as well, then you could write:


  string todo : todo != "" => skip == "";

Or, we could add an `implies' function to the standard library, and
then:

  string todo : implies (todo != "", skip == "");

What people think?

> +fun pktest_run_noexit = (PkTest[] tests, string skip = "") int:
> +  {
> +    var ok = 1;
> +    var i = 0UL;
> +
> +    if (skip != "")
> +      {
> +        printf "1..0 # Skipped: %s\n", skip;
> +        return ok;
> +      }
> +    else
> +      printf "1..%u64d\n", tests'length;
> +
> +    for (t in tests)
> +      {
> +        ++i;
> +
> +        if (t.skip != "")
> +          {
> +            printf "ok %u64d %s # SKIP %s\n", i, t.name, t.skip;
> +            continue;
> +          }
> +
> +        fun todo = void:
> +          {
> +            if (t.todo == "")
> +              print "\n";
> +            else
> +              printf " # TODO %s\n", t.todo;
> +          }
> +
> +        try t.func(t.name);
> +        catch (Exception ex)
> +          {
> +            ok = 0;
> +            printf "not ok %u64d %s: %s", i, t.name, ex.msg;
> +            todo ();
> +            continue;
> +          }
> +        printf "ok %u64d %s", i, t.name;
> +        todo ();
> +      }
> +
> +    return ok;
> +  }
> +
> +fun pktest_run = (PkTest[] tests, string skip = "") void:
> +  {
> +    exit (pktest_run_noexit (tests, skip) ? 0 : 1);
> +  }

I still think that pktest_run should not exit.  Writing

  exit (pktest_run (tests) ? 0 : 1);

is not that difficult nor cumbersome IMO.

This is a public API, so the smaller it is the better for everyone.
pktest_run runs a set of tests and returns a status.  What the user does
with it, is up to her.

Other than that, the test driver looks super to me! :)