Support for calls with no parameters

[tbrk]

Proposition for solving #169.

[mseri]

This actually looks fine. This "empty params" feels like a corner case of the spec to be fair, but so it is null which I think is an inconsistently supported extension.

Can you declare an "empty/void" call in tests/common/test_interface.ml and test it in tests/lib/client_server_test.ml to see what it looks like in practice?

[tbrk]

I agree. I had a very quick look last night to try to see what the spec said, but I don't think it considers this case. I guess the Python implementations used on servers define the spec 🤷.

[tbrk]

I've added those tests. They uncovered a bug in Codegen which I also fixed.

[mseri]

There are a few more fixes needed. If you run

dune build @runexamples -p rpc

You should be able to reproduce the issue.
All the generators need fixing

[tbrk]

Done! I've also added a makefile target to build/test the examples.

[tbrk]

Extended example: https://github.com/tbrk/neos-server-ocaml-client

[mseri]

One last thing, sorry. Can you update the changelog?

[mseri]

Thanks

[psafont]

Somehow this breaks the tests interfaces for xapi-idl. Was this meant to be a non-breaking change?

example:

7 | module C = Memory_interface.API (GenTestData (GenPath) (TJsonrpc))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: Modules do not match:
       sig
         type implementation = unit Alcotest.test_case list ref
         val tests : unit Alcotest.test_case list ref
         val description : Idl.Interface.description option ref
         val implement :
           Idl.Interface.description -> unit Alcotest.test_case list ref
         type ('a, 'b) comp = 'a
         type 'a res = unit
         type 'a fn =
           'a
           Dune__exe__Idl_test_common.GenTestData(GenPath)(Dune__exe.Idl_test_common.TJsonrpc).fn =
             Function : 'a Idl.Param.t * 'b fn -> ('a -> 'b) fn
           | Returning :
               ('a Idl.Param.t * 'b Idl.Error.t) -> ('a, 'c) comp fn
         val returning : 'a Idl.Param.t -> 'b Idl.Error.t -> ('a, 'c) comp fn
         val ( @-> ) : 'a Idl.Param.t -> 'b fn -> ('a -> 'b) fn
         val declare_ : bool -> string -> 'a -> 'b fn -> unit
         val declare : string -> 'a -> 'b fn -> unit
         val declare_notification : string -> 'a -> 'b fn -> unit
       end
     is not included in Idl.RPC
     The value `noargs' is required but not provided
     File "src/lib/idl.mli", line 107, characters 2-39: Expected declaration

[mseri]

Yes, this was meant as non-breaking (I would have waited and opted for a future 9 release othwerise). I hoped the tests would pick up any breakage.
Do you have some suggestions on re-implementing it in a backward compatible way?

[psafont]

Adding methods to signature is going to be a breaking change for code that reimplements interfaces for testing purposes, I think a similar case happened with the calls not needing a response.

In any case I'll change the code when I upgrade, I just wanted to make you aware of this ;)

[mseri]

It is a good point. I did not think carefully enough about that. Sorry again for the disruption and thanks for the notification

[tbrk]

Sorry for the hassle @psafont. I think @mseri was just trying to help me out with a problem I had using this library to call methods on a Python server. I wonder why the opam-repository regression testing didn't detect a problem with xapi-idl as a reverse dependency. (I looked back to the pull request but CI log is no longer available.)

[psafont]

xapi-idl is not on opam-repository and instead it's in a dedicated opam repository, so it's normal it got missed.

Now that most of the xapi code is in the same repository it should be possible to easily upload it to the main opam-repository. I don't think it would be useful for any other reason but to check for breaking dependencies,and it would add a significant load to the CI, and add burden to the repository maintainers.

I prefer the occasional hassle, it part of my job anyway :)

[tbrk]

OK. Thanks for the explanation!