use /bin/sh instead of /bin/false to make sure exit value is 1 (#2426)

Author: Vladimir Kotal

opengrok-tools/src/main/python/opengrok_tools/utils/commands.py

@@ -88,7 +88,8 @@ def run_command(self, command):
 
     def call_rest_api(self, command):
         """
-        Make RESTful API call.
+        Make RESTful API call. Occurrence of PROJECT_SUBST in the URI will be
+        replaced by project name.
         """
         command = command.get("command")
         uri = command[0].replace(self.PROJECT_SUBST, self.name)
@@ -97,7 +98,7 @@ def call_rest_api(self, command):
 
         headers = None
         json_data = None
-        if len(data) > 0:
+        if data:
             headers = {'Content-Type': 'application/json'}
             json_data = json.dumps(data).replace(self.PROJECT_SUBST, self.name)
             self.logger.debug("JSON data: {}".format(json_data))
@@ -109,7 +110,7 @@ def call_rest_api(self, command):
         elif verb == 'DELETE':
             delete(self.logger, uri, data)
         else:
-            self.logger.error('Unknown verb in command {}'.
+            self.logger.error('Unknown HTTP verb in command {}'.
                               format(command))
 
     def run(self):
@@ -119,6 +120,10 @@ def run(self):
         If the command has return code 2, the sequence will be terminated
         however it will not be treated as error.
 
+        If a command contains PROJECT_SUBST pattern, it will be replaced
+        by project name, otherwise project name will be appended to the
+        argument list of the command.
+
         Any command entry that is a URI, will be used to submit RESTful API
         request. Return codes for these requests are not checked.
         """

opengrok-tools/src/test/python/test_command.py

@@ -31,7 +31,7 @@
 
 sys.path.insert(0, os.path.abspath(
                 os.path.join(os.path.dirname(__file__), '..', '..',
-                'main', 'python', 'opengrok_tools')))
+                'main', 'python')))
 
 from opengrok_tools.utils.command import Command
 import tempfile

opengrok-tools/src/test/python/test_commands.py

@@ -30,7 +30,7 @@
 
 sys.path.insert(0, os.path.abspath(
                 os.path.join(os.path.dirname(__file__), '..', '..',
-                'main', 'python', 'opengrok_tools')))
+                'main', 'python')))
 
 from opengrok_tools.utils.commands import Commands, CommandsBase
 
@@ -41,50 +41,38 @@ def test_str(self):
                         [{"command": ['foo']}, {"command": ["bar"]}]))
         self.assertEqual("opengrok-master", str(cmds))
 
-    @unittest.skipUnless(os.path.exists('/bin/true') and os.path.exists('/bin/false'), "requires Unix")
+    @unittest.skipUnless(os.path.exists('/bin/sh') and os.path.exists('/bin/echo'), "requires Unix")
     def test_run_retcodes(self):
         cmds = Commands(CommandsBase("opengrok-master",
                         [{"command": ["/bin/echo"]},
-                         {"command": ["/bin/true"]},
-                         {"command": ["/bin/false"]}]))
+                         {"command": ["/bin/sh", "-c", "echo " + Commands.PROJECT_SUBST + "; exit 0"]},
+                         {"command": ["/bin/sh", "-c", "echo " + Commands.PROJECT_SUBST + "; exit 1"]}]))
         cmds.run()
-        # print(p.retcodes)
         self.assertEqual({'/bin/echo opengrok-master': 0,
-                          '/bin/true opengrok-master': 0,
-                          '/bin/false opengrok-master': 1}, cmds.retcodes)
+                          '/bin/sh -c echo opengrok-master; exit 0': 0,
+                          '/bin/sh -c echo opengrok-master; exit 1': 1}, cmds.retcodes)
 
-    @unittest.skipUnless(os.path.exists('/usr/bin/true') and os.path.exists('/usr/bin/false'), "requires Unix")
-    def test_run_retcodes_usr(self):
-        cmds = Commands(CommandsBase("opengrok-master",
-                        [{"command": ["/bin/echo"]},
-                         {"command": ["/usr/bin/true"]},
-                         {"command": ["/usr/bin/false"]}]))
-        cmds.run()
-        # print(p.retcodes)
-        self.assertEqual({'/bin/echo opengrok-master': 0,
-                          '/usr/bin/true opengrok-master': 0,
-                          '/usr/bin/false opengrok-master': 1}, cmds.retcodes)
-
-    @unittest.skipUnless(os.path.exists('/bin/true') and os.path.exists('/bin/false'), "requires Unix")
+    @unittest.skipUnless(os.path.exists('/bin/sh') and os.path.exists('/bin/echo'), "requires Unix")
     def test_terminate_after_non_zero_code(self):
         cmds = Commands(CommandsBase("opengrok-master",
-                                     [{"command": ["/bin/false"]},
-                                      {"command": ["/bin/true"]}]))
+                                     [{"command": ["/bin/sh", "-c", "echo " + Commands.PROJECT_SUBST + "; exit 255"]},
+                                      {"command": ["/bin/echo"]}]))
         cmds.run()
-        self.assertEqual({'/bin/false opengrok-master': 1}, cmds.retcodes)
+        self.assertEqual({'/bin/sh -c echo opengrok-master; exit 255': 255}, cmds.retcodes)
 
-    @unittest.skipUnless(os.path.exists('/usr/bin/true') and os.path.exists('/usr/bin/false'), "requires Unix")
-    def test_terminate_after_non_zero_code_usr(self):
+    @unittest.skipUnless(os.path.exists('/bin/sh') and os.path.exists('/bin/echo'), "requires Unix")
+    def test_exit_2_handling(self):
         cmds = Commands(CommandsBase("opengrok-master",
-                                     [{"command": ["/usr/bin/false"]},
-                                      {"command": ["/usr/bin/true"]}]))
+                                     [{"command": ["/bin/sh", "-c", "echo " + Commands.PROJECT_SUBST + "; exit 2"]},
+                                      {"command": ["/bin/echo"]}]))
         cmds.run()
-        self.assertEqual({'/usr/bin/false opengrok-master': 1}, cmds.retcodes)
+        self.assertEqual({'/bin/sh -c echo opengrok-master; exit 2': 2}, cmds.retcodes)
+        self.assertFalse(cmds.failed)
 
-    @unittest.skipUnless(os.name.startswith("posix"), "requires Unix")
+    @unittest.skipUnless(os.path.exists('/bin/echo'), "requires Unix")
     def test_project_subst(self):
         cmds = Commands(CommandsBase("test-subst",
-                        [{"command": ["/bin/echo", '%PROJECT%']}]))
+                        [{"command": ["/bin/echo", Commands.PROJECT_SUBST]}]))
         cmds.run()
         self.assertEqual(['test-subst\n'],
                          cmds.outputs['/bin/echo test-subst'])