[ovs-dev] ovsdb-tool: Skip monitor argument on Windows

Message ID 20180416132927.1480-1-aserdean@ovn.org
State Changes Requested
Headers show
Series
  • [ovs-dev] ovsdb-tool: Skip monitor argument on Windows
Related show

Commit Message

Alin Gabriel Serdean April 16, 2018, 1:29 p.m.
The daemon argument '--monitor' does not exist on Windows so the process will
complain:
+2018-04-16T13:10:24Z|00001|getopt_long|WARN|unknown option -- monitor
+ovsdb-server: Failed to read from child (The pipe has been ended.
+)
Skip the argument when running the test under Windows.

Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
---
 tests/ovsdb-tool.at | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

Ben Pfaff April 16, 2018, 8:11 p.m. | #1
On Mon, Apr 16, 2018 at 04:29:27PM +0300, Alin Gabriel Serdean wrote:
> The daemon argument '--monitor' does not exist on Windows so the process will
> complain:
> +2018-04-16T13:10:24Z|00001|getopt_long|WARN|unknown option -- monitor
> +ovsdb-server: Failed to read from child (The pipe has been ended.
> +)
> Skip the argument when running the test under Windows.
> 
> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>

Thanks for fixing the problem.

I think that we should remove --monitor unconditionally here, not just
on Windows.  At most, it will cover up some genuine bug that the tests
should find.

Thanks,

Ben.
Alin Gabriel Serdean April 16, 2018, 8:15 p.m. | #2
> On 16 Apr 2018, at 23:11, Ben Pfaff <blp@ovn.org> wrote:
> 
> On Mon, Apr 16, 2018 at 04:29:27PM +0300, Alin Gabriel Serdean wrote:
>> The daemon argument '--monitor' does not exist on Windows so the process will
>> complain:
>> +2018-04-16T13:10:24Z|00001|getopt_long|WARN|unknown option -- monitor
>> +ovsdb-server: Failed to read from child (The pipe has been ended.
>> +)
>> Skip the argument when running the test under Windows.
>> 
>> Signed-off-by: Alin Gabriel Serdean <aserdean@ovn.org>
> 
> Thanks for fixing the problem.
> 
> I think that we should remove --monitor unconditionally here, not just
> on Windows.  At most, it will cover up some genuine bug that the tests
> should find.
> 
> Thanks,
> 
> Ben.

Sounds good. I’ll send another revision.

Alin.

Patch

diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index ab766be24..7df7b6b43 100644
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -443,7 +443,11 @@  AT_CHECK(
 ]], [ignore])
 
 # Dump the data.
-AT_CHECK([ovsdb-server -vfile -vvlog:off --monitor --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db1])
+if test "$IS_WIN32" = "no"; then
+  AT_CHECK([ovsdb-server -vfile -vvlog:off --monitor --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db1])
+else
+  AT_CHECK([ovsdb-server -vfile -vvlog:off --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db1])
+fi
 AT_CHECK([ovsdb-client dump > expout])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
@@ -451,7 +455,11 @@  OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 ovsdb-tool create-cluster db2 db1 unix:s1.raft
 
 # Dump the data.
-AT_CHECK([ovsdb-server -vconsole:off -vfile -vvlog:off --monitor --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db2])
+if test "$IS_WIN32" = "no"; then
+  AT_CHECK([ovsdb-server -vconsole:off -vfile -vvlog:off --monitor --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db2])
+else
+  AT_CHECK([ovsdb-server -vconsole:off -vfile -vvlog:off --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db2])
+fi
 AT_CHECK([ovsdb-client wait ordinals connected])
 AT_CHECK([ovsdb-client dump > dump2])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])