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

[Mohammad-Reza Nabipoor]

Hi, Jose.

On Wed, Dec 16, 2020 at 12:43:39PM +0100, Jose E. Marchesi wrote:
> 
> 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 == "";
> 

This operator seems useful and much more readable than the other form.
I think this kind of contraints happen in real-world and having a readable
operator for describing them is good.
Implementing it is also not difficult.


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

I think exiting with the right status code is easy to forget. And failed
test script should not return 0.

And the reason for changing the `pktest_run` to exit is the output.
When the user invokes `pktest_run`, it writes test numbers to stdout.
I guess the user should not call `pktest_run` again hence make it
default to exit (because it can confuses the test harness).

But if you still think it's not useful. I'll remove it.

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

Thanks :)


Regards,
Mohammad-Reza