More pr recommendations implemented.

1 files changed, 46 insertions, 34 deletions

diff --git a/tools/python/integration_tests_runner.py b/tools/python/integration_tests_runner.py
index fd80c35f55..a5035d8ee5 100644
--- a/tools/python/integration_tests_runner.py
+++ b/tools/python/integration_tests_runner.py
@@ -1,15 +1,16 @@
 from __future__ import print_function
 
 import logging
+from contextlib import contextmanager
 from multiprocessing import cpu_count
 import multiprocessing
-from argparse import ArgumentParser
+from optparse import OptionParser
 from os import path
+import shutil
 import subprocess
-from warnings import warn
+import tempfile
 
 from run_desktop_tests import tests_on_disk
-from Util import TemporaryDirectory
 
 __author__ = 't.danshin'
 
@@ -21,6 +22,15 @@ DATA_PATH_KEY = "--data_path"
 RESOURCE_PATH_KEY = "--user_resource_path"
 
 
+@contextmanager
+def TemporaryDirectory():
+    name = tempfile.mkdtemp()
+    try:
+        yield name
+    finally:
+        shutil.rmtree(name)
+
+
 def setup_test_result_log(log_file, level=logging.INFO):
     logger = logging.getLogger(TEST_RESULT_LOG)
     formatter = logging.Formatter("BEGIN: %(file)s\n%(message)s\nEND: %(file)s | result: %(result)d\n")
@@ -64,9 +74,8 @@ def spawn_test_process(test, flags):
 
     multiprocessing.get_logger().info(spell[0])
     out, err = process.communicate()
-    #We need the out for getting the list of tests from an exec file
-    # by sending it the --list_tests flag
-    return test, filter(None, out.splitlines()), err, process.returncode
+
+    return filter(None, out.splitlines()), err, process.returncode
 
 
 def exec_test(a_tuple):
@@ -85,10 +94,14 @@ def exec_test(a_tuple):
     test_file, test_name, params = a_tuple
     params[FILTER_KEY] = test_name
 
-    if path.basename(test_file) in TEMPFOLDER_TESTS:
-        return exec_test_with_temp(test_file, params)
+    out, err, result = None, None, None
+
+    if test_file in TEMPFOLDER_TESTS:
+        out, err, result = exec_test_with_temp(test_file, params)
+    else:
+        out, err, result = exec_test_without_temp(test_file, params)
 
-    return exec_test_without_temp(test_file, params)
+    return (test_file, test_name), (err, result)
 
 
 def exec_test_with_temp(test_file, params):
@@ -96,7 +109,6 @@ def exec_test_with_temp(test_file, params):
         params[DATA_PATH_KEY] = tmpdir
         return exec_test_without_temp(test_file, params)
 
-
 def exec_test_without_temp(test_file, params):
     flags = params_from_dict(params)
     return spawn_test_process(test_file, flags)
@@ -134,7 +146,7 @@ class IntegrationRunner:
                 exec_test,
                 self.prepare_list_of_tests()
             )
-            for test_file, _, err, result in test_results:
+            for (test_file, _), (err, result) in test_results:
                 logger.info(
                     err,
                     extra={"file" : path.basename(test_file), "result" : result}
@@ -147,7 +159,7 @@ class IntegrationRunner:
 
     def map_args(self, test):
         test_full_path = path.join(self.workspace_path, test)
-        tests = spawn_test_process(test_full_path, "--list_tests")[1]  # a list
+        tests = spawn_test_process(test_full_path, "--list_tests")[0]  # a list
 
         return map(
             lambda tests_in_file: (test_full_path, tests_in_file, self.params),
@@ -161,66 +173,66 @@ class IntegrationRunner:
                 yield test_tuple
 
 
-    def set_instance_vars_from_options(self, args):
-        self.workspace_path = args.folder
-        for opt in args.runlist:
+    def set_instance_vars_from_options(self, options):
+        self.workspace_path = options.folder
+        for opt in options.runlist:
             self.runlist.extend(map(str.strip, opt.split(",")))
 
         tests_on_disk_list = tests_on_disk(self.workspace_path)
         self.runlist = filter(lambda x: x in tests_on_disk_list, self.runlist)
 
-        self.params[RESOURCE_PATH_KEY] = args.user_resource_path
-        self.params[DATA_PATH_KEY] = args.data_path
+        self.params[RESOURCE_PATH_KEY] = options.user_resource_path
+        self.params[DATA_PATH_KEY] = options.data_path
 
 
     def process_cli(self):
         parser = self.prepare_cli_parser()
-        args = parser.parse_args()
+        (options, args) = parser.parse_args()
+
+        if not options.runlist:
+            parser.print_help()
+            raise Exception("You must provide a list of tests to run")
 
-        self.set_instance_vars_from_options(args)
+        self.set_instance_vars_from_options(options)
 
-        setup_test_result_log(args.output)
+        setup_test_result_log(options.output)
         setup_jenkins_console_logger()
 
-        if args.log_start_finish:
-            warn(
-                "The -l option is now deprecated. Please, remove it from your build scripts. It may be removed at any time.",
-                 DeprecationWarning
-            )
+        if options.log_start_finish:
+            multiprocessing.get_logger().warning("The -l option is now deprecated. Please, remove it from your build scripts. It may be removed at any time.")
 
 
     def prepare_cli_parser(self):
-        parser = ArgumentParser()
-        parser.add_argument(
+        parser = OptionParser()
+        parser.add_option(
             "-o", "--output",
             dest="output", default="testlog.log",
             help="resulting log file. Default testlog.log"
         )
-        parser.add_argument(
+        parser.add_option(
             "-f", "--folder",
             dest="folder", default="omim-build-release/out/release",
             help="specify the folder where the tests reside (absolute path or relative to the location of this script)"
         )
-        parser.add_argument(
+        parser.add_option(
             "-i", "--include",
-            required=True,
             dest="runlist", action="append", default=[],
             help="Include test into execution, comma separated list with no spaces or individual tests, or both. E.g.: -i one -i two -i three,four,five"
         )
-        parser.add_argument(
+        parser.add_option(
             "-r", "--user_resource_path",
             dest="user_resource_path", default="",
             help="Path to user resources, such as MWMs"
         )
-        parser.add_argument(
+        parser.add_option(
             "-d", "--data_path",
             dest="data_path", default="",
             help="Path to the writable dir"
         )
-        parser.add_argument(
+        parser.add_option(
             "-l", "--log_start_finish",
             dest="log_start_finish", action="store_true", default=False,
-            help="DEPRECATED. Write to log each time a test starts or finishes. May be useful if you need to find out which of the tests runs for how long, and which test hang. May slow down the execution of tests."
+            help="Write to log each time a test starts or finishes. May be useful if you need to find out which of the tests runs for how long, and which test hang. May slow down the execution of tests."
         )
         return parser