Fix exit value of `update` command.

It was calling `exit 1` on sucess, because it expected the various `update_*`
functions to return booleans. They don't. It looks like they return undef on
success and `exit` themselves on failure.

So change the code to only exit with failure if the thing we're asked
to update is unknown. Ottherwise, call the proper `update_*` function
and `exit 0`.

Discovered by changing the `ctl` test method die if `qx()` exits abnormally
and starting to check the return value of `ctl` more often.

diff --git a/bucardo b/bucardo
index 84b2b5092b5282a2ccd3209bd160730eeefe442e..d4fce2f3049f50546afcc4ec408b015cbda4bcad 100755 (executable)
--- a/bucardo
+++ b/bucardo
@@ -1363,20 +1363,18 @@ sub update_item {
     ## Account for variations and abbreviations
     $thing = standardize_name($thing);
 
-    ## These do return due to recursion, so we must exit afterwards
-    (update_customcode() or exit 1) if $thing eq 'customcode';
-    ## We do not update customname or customcols
-    ## The dbgroup must be checked before the database (dbg vs db)
-    (update_database(@nouns) or exit 1) if $thing eq 'database';
-    (update_dbgroup(@nouns)  or exit 1) if $thing eq 'dbgroup';
-    (update_herd(@nouns)     or exit 1) if $thing eq 'herd';
-    (update_sync(@nouns)     or exit 1) if $thing eq 'sync';
-    (update_table(@nouns)    or exit 1) if $thing eq 'table' or $thing eq 'sequence';
-
-    ## If we got this far, we have an unknown thing
-    warn "$usage\n";
-    exit 1;
-
+    my $code = $thing eq 'customcode' ? \&update_customcode
+             : $thing eq 'database'   ? \&update_database
+             : $thing eq 'dbgroup'    ? \&update_dbgroup
+             : $thing eq 'herd'       ? \&update_herd
+             : $thing eq 'sync'       ? \&update_sync
+             : $thing eq 'table'      ? \&update_table
+             : $thing eq 'sequence'   ? \&update_table
+             :                          do { warn "$usage\n"; exit 1; };
+
+    ## The update function returns, due to recursion, so we must exit.
+    $code->(@nouns);
+    exit 0;
 } ## end of update_item
 
 
@@ -10204,14 +10202,14 @@ How long it has been since the last sync failure
 
 =item B<activate>
 
-  bucardo syncname [syncname2 syncname3 ...] [timeout]
+  bucardo activate syncname [syncname2 syncname3 ...] [timeout]
 
 Activates one or more named syncs. If given a timeout argument, it will wait until it has received 
 confirmation from Bucardo that each sync has been successfully activated.
 
 =item B<deactivate>
 
-  bucardo syncname [syncname2 syncname3 ...] [timeout]
+  bucardo deactivate syncname [syncname2 syncname3 ...] [timeout]
 
 Deactivates one or more named syncs. If given a timeout argument, it will wait until it has received 
 confirmation from Bucardo that the sync has been successfully deactivated.

diff --git a/t/20-postgres.t b/t/20-postgres.t
index 65580f6d6f13f19343ece09b3dcad0cd3f9592fc..ceba4ab2c3199158c80ab84f43f834cb31104c7f 100644 (file)
--- a/t/20-postgres.t
+++ b/t/20-postgres.t
@@ -200,10 +200,12 @@ $res = $dbhA->selectall_arrayref($SQL);
 is_deeply($res, [[1]], $t);
 
 ## Switch to a 2 source sync
-$bct->ctl('bucardo update sync pgtest1 status=inactive');
-$bct->ctl('bucardo update sync pgtest5 status=active');
-$bct->ctl('bucardo deactivate sync pgtest1');
-$bct->ctl('bucardo activate sync pgtest5 0');
+is $bct->ctl('bucardo update sync pgtest1 status=inactive'), '', 'Set pgtest1 status=inactive';
+is $bct->ctl('bucardo update sync pgtest5 status=active'), '', 'Set pgtest5 status=active';
+is $bct->ctl('bucardo deactivate pgtest1'), "Deactivating sync pgtest1\n",
+    'Deactivate pgtest1';
+is $bct->ctl('bucardo activate pgtest5 0'), "Activating sync pgtest5...OK\n",
+    'Activate pgtest5';
 ## Add some rows to both masters, make sure it goes everywhere
 $bct->add_row_to_database('A', 3);
 $bct->add_row_to_database('B', 4);
@@ -211,7 +213,7 @@ $bct->add_row_to_database('B', 4);
 ## Kick off the sync.
 $bct->ctl('bucardo kick sync pgtest5 0');
 
-## All rows should be on A, B, C, and D
+## All rows should be on A and B.
 my $expected = [[1],[3],[4]];
 $bct->check_for_row($expected, [qw/A B/]);
 

diff --git a/t/BucardoTesting.pm b/t/BucardoTesting.pm
index aca50444e61d667fbe2d8335aa8c60c6a1859499..21631ed4393df4ac63bd168064910aae7dd78724 100644 (file)
--- a/t/BucardoTesting.pm
+++ b/t/BucardoTesting.pm
@@ -913,7 +913,7 @@ sub ctl {
     $args =~ s/^\s+//s;
 
     ## Allow the caller to look better
-    $args =~ s/^bucardo//;
+    $args =~ s/^bucardo\s+//;
 
     ## Set a timeout
     alarm 0;
@@ -922,6 +922,8 @@ sub ctl {
         alarm $ALARM_BUCARDO;
         debug("Connection options: $connopts Args: $args", 3);
         $info = qx{$ctl $connopts $args 2>&1};
+        debug("Exit value: $?", 3);
+        die $info if $? != 0;
         alarm 0;
     };