[PATCH] ctdb-test: Add tests to verify queue processing
martin at meltin.net
Tue Mar 26 09:43:46 UTC 2019
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?
> 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,
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Size: 833 bytes
Desc: OpenPGP digital signature
More information about the samba-technical