| Message ID | 1464781514-9508-1-git-send-email-pboca@cloudbasesolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On 1 June 2016 at 04:45, Paul Boca <pboca@cloudbasesolutions.com> wrote: > On Windows trying to overwrite the opened ovs-vswitchd.log > fails with access denied. Closing it before trying to overwrite it > solves the problem > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> Ideally we don't introduce platform differences into the tests; the extra command should be available on both platforms, so you should be able to just add "ovs-appctl vlog/close" rather than introducing the if/else. If you don't have a linux environment to test, then you can push the patches to your github and get travis to try it for you.
Hi! Will send a new version for this tomorrow with just the extra command. Thanks, Paul > -----Original Message----- > From: Joe Stringer [mailto:joe@ovn.org] > Sent: Friday, June 3, 2016 1:56 AM > To: Paul Boca > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH V2 1/4] tests: Fixed access denied on ovs- > vswitchd.log > > On 1 June 2016 at 04:45, Paul Boca <pboca@cloudbasesolutions.com> wrote: > > On Windows trying to overwrite the opened ovs-vswitchd.log > > fails with access denied. Closing it before trying to overwrite it > > solves the problem > > > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > Ideally we don't introduce platform differences into the tests; the > extra command should be available on both platforms, so you should be > able to just add "ovs-appctl vlog/close" rather than introducing the > if/else. > > If you don't have a linux environment to test, then you can push the > patches to your github and get travis to try it for you.
On Thu, Jun 02, 2016 at 03:55:41PM -0700, Joe Stringer wrote: > On 1 June 2016 at 04:45, Paul Boca <pboca@cloudbasesolutions.com> wrote: > > On Windows trying to overwrite the opened ovs-vswitchd.log > > fails with access denied. Closing it before trying to overwrite it > > solves the problem > > > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > Ideally we don't introduce platform differences into the tests; the > extra command should be available on both platforms, so you should be > able to just add "ovs-appctl vlog/close" rather than introducing the > if/else. > > If you don't have a linux environment to test, then you can push the > patches to your github and get travis to try it for you. Or for something as simple as this, just note in the change log that you only tested on Windows, and then whoever reviews it should run the test locally (I often do this in any case).
Hi! I did ran the test on Linux but failed saying vlog/close not recognized, maybe it was my mistake and didn't updated the sources. I will update and retest before submitting the next version. Regards, Paull > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Friday, June 3, 2016 2:05 AM > To: Joe Stringer > Cc: Paul Boca; dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH V2 1/4] tests: Fixed access denied on ovs- > vswitchd.log > > On Thu, Jun 02, 2016 at 03:55:41PM -0700, Joe Stringer wrote: > > On 1 June 2016 at 04:45, Paul Boca <pboca@cloudbasesolutions.com> wrote: > > > On Windows trying to overwrite the opened ovs-vswitchd.log > > > fails with access denied. Closing it before trying to overwrite it > > > solves the problem > > > > > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > > > Ideally we don't introduce platform differences into the tests; the > > extra command should be available on both platforms, so you should be > > able to just add "ovs-appctl vlog/close" rather than introducing the > > if/else. > > > > If you don't have a linux environment to test, then you can push the > > patches to your github and get travis to try it for you. > > Or for something as simple as this, just note in the change log that you > only tested on Windows, and then whoever reviews it should run the test > locally (I often do this in any case).
vlog/close isn't Windows-only, so I guess that something else must have happened. On Thu, Jun 02, 2016 at 11:10:03PM +0000, Paul Boca wrote: > I did ran the test on Linux but failed saying vlog/close not recognized, > maybe it was my mistake and didn't updated the sources. > I will update and retest before submitting the next version. > > Regards, > Paull > > > -----Original Message----- > > From: Ben Pfaff [mailto:blp@ovn.org] > > Sent: Friday, June 3, 2016 2:05 AM > > To: Joe Stringer > > Cc: Paul Boca; dev@openvswitch.org > > Subject: Re: [ovs-dev] [PATCH V2 1/4] tests: Fixed access denied on ovs- > > vswitchd.log > > > > On Thu, Jun 02, 2016 at 03:55:41PM -0700, Joe Stringer wrote: > > > On 1 June 2016 at 04:45, Paul Boca <pboca@cloudbasesolutions.com> wrote: > > > > On Windows trying to overwrite the opened ovs-vswitchd.log > > > > fails with access denied. Closing it before trying to overwrite it > > > > solves the problem > > > > > > > > Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> > > > > > > Ideally we don't introduce platform differences into the tests; the > > > extra command should be available on both platforms, so you should be > > > able to just add "ovs-appctl vlog/close" rather than introducing the > > > if/else. > > > > > > If you don't have a linux environment to test, then you can push the > > > patches to your github and get travis to try it for you. > > > > Or for something as simple as this, just note in the change log that you > > only tested on Windows, and then whoever reviews it should run the test > > locally (I often do this in any case).
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index bf9cf88..f65d391 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -6923,7 +6923,11 @@ OVS_WAIT_UNTIL([grep "monitor thread created" ovs-vswitchd.log]) AT_CHECK([ovs-vsctl set interface p0 bfd:enable=false]) # check log. OVS_WAIT_UNTIL([grep "monitor thread terminated" ovs-vswitchd.log]) -AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +if test "$IS_WIN32" = "no"; then + AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +else + AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && ovs-appctl.exe vlog/close && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +fi # enable cfm on p0. AT_CHECK([ovs-vsctl set interface p0 cfm_mpid=10]) @@ -6933,7 +6937,11 @@ OVS_WAIT_UNTIL([grep "monitor thread created" ovs-vswitchd.log]) AT_CHECK([ovs-vsctl remove interface p0 cfm_mpid 10]) # check log. OVS_WAIT_UNTIL([grep "monitor thread terminated" ovs-vswitchd.log]) -AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +if test "$IS_WIN32" = "no"; then + AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +else + AT_CHECK([sed -e '/^.*ofproto_dpif_monitor.*$/d' < ovs-vswitchd.log > tmp && ovs-appctl.exe vlog/close && mv tmp ovs-vswitchd.log && ovs-appctl vlog/reopen]) +fi # enable both bfd and cfm on p0. AT_CHECK([ovs-vsctl set interface p0 bfd:enable=true cfm_mpid=10])
On Windows trying to overwrite the opened ovs-vswitchd.log fails with access denied. Closing it before trying to overwrite it solves the problem Signed-off-by: Paul-Daniel Boca <pboca@cloudbasesolutions.com> --- tests/ofproto-dpif.at | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)