[Libguestfs] [PATCH libnbd v2 1/9] golang: tests: Add test for AioBuffer

[Richard W.M. Jones]

On Sat, Feb 12, 2022 at 01:05:29AM +0200, Nir Soffer wrote:
> On Fri, Feb 11, 2022 at 9:08 PM Nir Soffer <nsoffer at redhat.com> wrote:
> >
> > On Fri, Feb 11, 2022 at 1:22 PM Richard W.M. Jones <rjones at redhat.com> wrote:
> > >
> > > On Fri, Feb 11, 2022 at 03:21:21AM +0200, Nir Soffer wrote:
> > > > Add unit tests and benchmarks for AioBuffer. The tests are trivial but
> > > > they server as running documentation, and they point out important
> > > > details about the type.
> > > >
> > > > The benchmarks show the efficiency of allocating a new buffer, zeroing
> > > > it, and interfacing with Go code.
> > > >
> > > > These tests will also ensure that we don't break anything by the next
> > > > changes.
> > > >
> > > > To run the benchmarks use:
> > > >
> > > > $ go test -run=xxx -bench=.
> > > [...]
> > > > +# Run the benchmarks with 10 milliseconds timeout to make sure they do
> > > > +# not break by mistake, without overloading the CI. For performance
> > > > +# testing run "go test" directly.
> > > > +$GOLANG test -run=XXX -bench=. -benchtime=10ms
> > >
> > > -run param is a regexp matching the names of the tests to run.  It
> > > might be best to use something like this instead:
> > >
> > >   go test -run=<regexp> -bench=.
> > >
> > > because elsewhere we use "XXX" to mark code that needs to be fixed.
> >
> > The intent of this command is to run only the benchmark, using -run=XXX
> > to match no test. I agree this is a poor choice for this project since we use
> > XXX for other purposes.
> >
> > >
> > > Apart from this the whole series seems fine to me, ACK.
> >
> > Thanks, I'll push this with a better regex.
> 
> Updated using this change:
> 
> $ git diff -U3 aio-buffer-v2..
> diff --git a/golang/run-tests.sh b/golang/run-tests.sh
> index 3a07b23a..82dcca56 100755
> --- a/golang/run-tests.sh
> +++ b/golang/run-tests.sh
> @@ -28,7 +28,9 @@ requires nbdkit --version
>  # The -v option enables verbose output.
>  $GOLANG test -count=1 -v
> 
> -# Run the benchmarks with 10 milliseconds timeout to make sure they do
> -# not break by mistake, without overloading the CI. For performance
> +# Run the only benchmarks with 10 milliseconds timeout to make sure they
> +# do not break by mistake, without overloading the CI. For performance
>  # testing run "go test" directly.
> -$GOLANG test -run=XXX -bench=. -benchtime=10ms
> +# The -run=- parameter is the way to match no test, assuming that no test or
> +# sub test contains "-" in the name.
> +$GOLANG test -run=- -bench=. -benchtime=10ms
> 
> I'm waiting a few days in case Eric wants to comment.

I admit that I misread the original patch and thought this was a
comment (with "XXX" as a placeholder), rather than a command that
runs.  I think your updated version is better.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top