Richard W.M. Jones
2022-May-05 08:48 UTC
[Libguestfs] [libguestfs PATCH 4/4] tests/regressions: remove "iface"-based restrictions
On Thu, May 05, 2022 at 08:25:35AM +0200, Laszlo Ersek wrote:> On 05/04/22 20:50, Richard W.M. Jones wrote: > > > > For the whole series: > > > > Reviewed-by: Richard W.M. Jones <rjones at redhat.com> > > > > Do you think it makes sense to completely drop the two regression > > tests in patch 4? They're testing something that (probably) cannot go > > wrong now. > > We can do that, but then a few more test cases should be modified > perhaps, for consistency. If you grep the tree for "iface", you'll find > a few test cases that pass various "iface" params (off-hand I recall > Ruby, OCaml, and Java-language test cases -- I think those are all > "bindtests"). I figured those tests still made sense (for ensuring API > compat), so I didn't remove the iface params, and/or the test cases that > only differrerd from the rest in their iface params. And then I figured > the rhbz regression tests should stay too (just be relaxed).We cannot drop the parameter, it's part of the ABI.> If we drop the latter (the rhbz regression tests), should we still keep > the other (language bindings) tests?The bindtests are mainly about testing the binding between language and the C library so without actually looking IIRC 'iface' is only used to check that strings are converted correctly. Anyway, it's fine to leave it. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit
Laszlo Ersek
2022-May-05 11:06 UTC
[Libguestfs] [libguestfs PATCH 4/4] tests/regressions: remove "iface"-based restrictions
On 05/05/22 10:48, Richard W.M. Jones wrote:> On Thu, May 05, 2022 at 08:25:35AM +0200, Laszlo Ersek wrote: >> On 05/04/22 20:50, Richard W.M. Jones wrote: >>> >>> For the whole series: >>> >>> Reviewed-by: Richard W.M. Jones <rjones at redhat.com> >>> >>> Do you think it makes sense to completely drop the two regression >>> tests in patch 4? They're testing something that (probably) cannot go >>> wrong now. >> >> We can do that, but then a few more test cases should be modified >> perhaps, for consistency. If you grep the tree for "iface", you'll find >> a few test cases that pass various "iface" params (off-hand I recall >> Ruby, OCaml, and Java-language test cases -- I think those are all >> "bindtests"). I figured those tests still made sense (for ensuring API >> compat), so I didn't remove the iface params, and/or the test cases that >> only differrerd from the rest in their iface params. And then I figured >> the rhbz regression tests should stay too (just be relaxed). > > We cannot drop the parameter, it's part of the ABI.I didn't mean dropping the parameter; I meant dropping the testing of the parameter in the bind tests. However:>> If we drop the latter (the rhbz regression tests), should we still keep >> the other (language bindings) tests? > > The bindtests are mainly about testing the binding between language > and the C library so without actually looking IIRC 'iface' is only > used to check that strings are converted correctly.You are right that as long as the parameter is part of the API (regardless of whether it is used for anything useful, or just ignored), we should continue testing *its binding*.> Anyway, it's fine to leave it.Commit range 4864d21cb8eb..ddf276884c04. Thank you! Laszlo