Giuseppe Lavagetto has submitted this change and it was merged.

Change subject: Have confctl exit with status code 1 if one action fails.
......................................................................


Have confctl exit with status code 1 if one action fails.

Change-Id: Ib389e31beff8ac758848c24a54014c9c5718ced3
---
M conftool/cli/tool.py
M conftool/tests/integration/test_tool.py
2 files changed, 25 insertions(+), 10 deletions(-)

Approvals:
  Giuseppe Lavagetto: Looks good to me, approved
  Volans: Looks good to me, but someone else must approve
  BBlack: Looks good to me, but someone else must approve
  jenkins-bot: Verified



diff --git a/conftool/cli/tool.py b/conftool/cli/tool.py
index 7d850aa..63cad8f 100644
--- a/conftool/cli/tool.py
+++ b/conftool/cli/tool.py
@@ -113,6 +113,9 @@
                 print(msg)
         if not fail:
             self.announce()
+            return True
+        else:
+            return False
 
     @staticmethod
     def raise_warning():
@@ -282,9 +285,12 @@
         _log.critical("Invalid configuration: %s", e)
         sys.exit(1)
 
+    exit_status = 0
     for unit in args.action:
         # TODO: fix base class
-        cli.run_action(unit)
+        if not cli.run_action(unit):
+            exit_status = 1
+    sys.exit(exit_status)
 
 
 if __name__ == '__main__':
diff --git a/conftool/tests/integration/test_tool.py 
b/conftool/tests/integration/test_tool.py
index 8557115..90a027a 100644
--- a/conftool/tests/integration/test_tool.py
+++ b/conftool/tests/integration/test_tool.py
@@ -31,7 +31,9 @@
 
     def output_for(self, args):
         with captured_output() as (out, err):
-            tool.main(cmdline=args)
+            with self.assertRaises(SystemExit) as cm:
+                tool.main(cmdline=args)
+                self.assertEquals(cm.exception_code, 0)
         res = out.getvalue().strip()
         output = []
         if not res:
@@ -53,10 +55,7 @@
 
     def test_get_node(self):
         args = self.generate_args('get cp1008')
-        with captured_output() as (out, err):
-            tool.main(cmdline=args)
-        res = out.getvalue().strip()
-        output = json.loads(res)
+        output = self.output_for(args)[0]
         k = output.keys()
         k.sort()
         self.assertEquals(k, ['cp1008', 'tags'])
@@ -82,7 +81,9 @@
         Changing values according to a regexp
         """
         args = self.generate_args('set/pooled=yes re:cp105.')
-        tool.main(cmdline=args)
+        with self.assertRaises(SystemExit) as cm:
+            tool.main(cmdline=args)
+            self.assertEquals(cm.exception_code, 0)
         self.assertEquals(self.irc.emit.called, True)
         for hostname in ['cp1052', 'cp1053', 'cp1054', 'cp1055']:
             n = node.Node('eqiad', 'cache_text', 'https', hostname)
@@ -94,7 +95,10 @@
         Test creation is not possible from confctl
         """
         args = self.generate_args('set/pooled=yes cp1059')
-        tool.main(cmdline=args)
+        with self.assertRaises(SystemExit) as cm:
+            tool.main(cmdline=args)
+            # In error, we exit with status 1
+            self.assertEquals(cm.exception_code, 1)
         # This doesn't get announced to irc.
         self.assertEquals(self.irc.emit.called, False)
         n = node.Node('eqiad', 'cache_text', 'https', 'cp1059')
@@ -114,7 +118,9 @@
         # one node, or not if we don't
         args = ['select', 'cluster=appservers', 'set/pooled=yes']
         tool.ToolCliByLabel.raise_warning = mock.MagicMock()
-        tool.main(cmdline=args)
+        with self.assertRaises(SystemExit) as cm:
+            tool.main(cmdline=args)
+            self.assertEquals(cm.exception_code, 0)
         self.assertEquals(tool.ToolCliByLabel.raise_warning.call_count,1)
         # now let's loop through the responses from conftool get
         args = ['select', 'cluster=appservers', 'get']
@@ -125,7 +131,10 @@
             self.assertEquals(res[k]['pooled'], 'yes')
         tool.ToolCliByLabel.raise_warning.reset_mock()
         args = ['select', 'name=mw1018', 'set/pooled=inactive']
-        tool.main(cmdline=args)
+        with self.assertRaises(SystemExit) as cm:
+            tool.main(cmdline=args)
+            self.assertEquals(cm.exception_code, 0)
+
         tool.ToolCliByLabel.raise_warning.assert_not_called()
         out = self.output_for(['select', 'name=mw1018', 'get'])
         self.assertEquals(out[0]['mw1018']['pooled'], 'inactive')

-- 
To view, visit https://gerrit.wikimedia.org/r/311678
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib389e31beff8ac758848c24a54014c9c5718ced3
Gerrit-PatchSet: 3
Gerrit-Project: operations/software/conftool
Gerrit-Branch: master
Gerrit-Owner: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: BBlack <bbl...@wikimedia.org>
Gerrit-Reviewer: Ema <e...@wikimedia.org>
Gerrit-Reviewer: Giuseppe Lavagetto <glavage...@wikimedia.org>
Gerrit-Reviewer: Volans <rcocci...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to