On Mon, Mar 16, 2015 at 03:07:04PM -0700, Ben Pfaff wrote:
> On Fri, Mar 13, 2015 at 04:51:55PM -0700, Jarno Rajahalme wrote:
> > This added test should fail. Next patch fixes OVS to pass this test.
> 
> Thanks for adding tests!  More tests now mean easier work later.
> 
> It's not nice to have a commit that breaks tests, even if it's fixed
> by the following commit; it can cause trouble in bisecting, for
> example.  So, I would usually add a commit like this just *after* the
> commit that fixes the problem, or combine it with the commit that does
> the fix.
> 
> > +AT_CHECK([ovs-appctl time/warp 500], [0],
> > +[warped
> > +])
> > +sleep 1  # wait for log writer
> 
> I assume that usually the log write would be done much more quickly
> than 1 second later?  Is it easy to replace this "sleep" by an
> OVS_WAIT_WHILE or OVS_WAIT_UNTIL?  It is nice to skip sleeps, if one
> can.
> 
> > +# The WARN was intended, remove it
> > +rm ovs-vswitchd.log
> > +touch ovs-vswitchd.log
> 
> It would be better, if it is possible, to instead skip this warning
> via the argument to OVS_VSWITCHD_STOP.
> 
> > +OVS_VSWITCHD_STOP
> > +AT_CLEANUP
> > +
> >  AT_SETUP([ofproto-dpif - port duration])
> >  OVS_VSWITCHD_START([set Bridge br0 protocols=OpenFlow13])
> >  ADD_OF_PORTS([br0], 1, 2)

Oh, also s/accross/across/ in the subject.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to