> On Mar 16, 2015, at 3:07 PM, Ben Pfaff <b...@nicira.com> 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. >
OK, I’ll squash this with the following patch. >> +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. > I’ll use this instead: OVS_WAIT_UNTIL([test `grep flow_add ovs-vswitchd.log | wc -l` -ge 1]) >> +# 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. > Thanks for the reminder, I’ll use this instead: OVS_VSWITCHD_STOP(["/Failed to pop from an empty stack/d"]) >> +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) > > Thanks, > > Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
