[PATCH] ctdb-test: Add tests to verify queue processing

[Martin Schwenke]

On Thu, 21 Mar 2019 11:05:29 +0100, swen <swen at linux.ibm.com> wrote:

> On Thu, 2019-03-21 at 17:15 +1100, Martin Schwenke wrote:

> > On Mon, 18 Mar 2019 15:34:30 +0100, swen <swen at linux.ibm.com> wrote:
> >   
> > > On Mon, 2019-03-18 at 21:58 +1100, Martin Schwenke wrote:  
>  [...]  
> > > Thanks for your reply.
> > > I integrated the patches into ctdb_io_test.c and performed all the
> > > other updates as well (ctdb_io_test_001.sh)
> > > 
> > > Please review and push if happy.  
> > 
> > I would suggest not changing test_setup() to *return* the queue,
> > while
> > dropping the ctdb output parameter.  That forces the queue it into
> > tests
> > that don't use it.
> > 
> > Instead, as I suggested in my previous email, I suggest adding an
> > optional output argument for the queue and only pass it out if non-
> > NULL
> > is passed in.  That way, the change to existing tests is minimal:
> > just
> > pass NULL for the extra argument.  This makes it clear that those
> > tests
> > aren't looking inside the queue.
> > 
> > The new tests could then follow the existing pattern of using ctdb-  
> > >ev  
> > for the event loop and using queue when necessary.
> > 
> > How does that sound?  
> Well, I would have preferred to "use" ctdb_io as it is by the real
> functional consumers in the code, meaning, to have the queue as the
> center of gravity.
> But I do see your point.
> 
> Therefore, I changed it to the way you prefer.
> 
> Please review and push if happy 
> ... you shoudld be totally excited now :-) 

Looks good.  Nice tests...

I'd probably suggest merging the enabling of the tests, currently in
patch 4, into patches 2 & 3.  There's nothing terribly exciting
happening in patch 4 and enabling the test in the script fits
into the 1 thing that the patches 2 & 3 are doing: adding a test.
However, I won't spend time arguing about it.  You could repost with
that change or not bother.  :-)

Depending on whether this happens, there's a minor typo in the commit
message of patch 3: s/poacket/packet/ .  If you repost without patch
4 as above then please fix it, otherwise the 2nd reviewer can fix
it.  When they add the Reviewed-by: tags.

Reviewed-by: Martin Schwenke <martin at meltin.net>

2nd team reviewer, please?

Thanks...

> P.S.: One thing I have to say.... I really, really dislike the
> hungarian notation, totally odd stuff, and I felt physical pain to not
> change it and even worse to add to it.

I had to look up "Hungarian notation".  Thanks for not changing it.
Christof obviously likes it...  ;-)

peace & happiness,
martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190326/97e8ee05/attachment.sig>