Added tests for interactive mode (#397)

* restructured query tests
* added tests for interactive mode
* commented out unused code

Author: Elliot Boschwitz

build.py

@@ -152,8 +152,8 @@ def unit_test():
     utility.exec_command(('pytest -l --cov mssqlcli --doctest-modules '
                           '--junitxml=junit/test-{}-results.xml --cov-report=xml '
                           '--cov-report=html --cov-append -o junit_suite_name=pytest-{} '
-                          '-s -m "not unstable" {}').format(runid, python_version,
-                                                            get_active_test_filepaths()),
+                          '-m "not unstable" {}').format(runid, python_version,
+                                                         get_active_test_filepaths()),
                          utility.ROOT_DIR, continue_on_error=False)
 
 def unstable_unit_test():
@@ -181,6 +181,7 @@ def get_active_test_filepaths():
         'tests/test_telemetry.py '
         'tests/test_localization.py '
         'tests/test_globalization.py '
+        'tests/test_interactive_mode.py '
         'tests/test_noninteractive_mode.py '
         'tests/test_special.py'
     )

mssqlcli/mssql_cli.py

@@ -192,37 +192,36 @@ def __init__(self, options):
 
         # exit and return error if user enters interactive mode with -i or -o arguments enabled
         if self.interactive_mode and (self.input_file or self.output_file):
-            click.secho("Invalid arguments: -i and -o can only be used in non-interactive mode.",
-                        err=True, fg='red')
-            sys.exit(1)
+            raise ValueError("Invalid arguments: -i and -o can only be used in non-interactive "
+                             "mode.")
 
         # exit and return error if both query text and an input file are specified
         if self.query and self.input_file:
-            click.secho("Invalid arguments: either query [-Q] or input file [-i] may be specified.",
-                        err=True, fg='red')
-            sys.exit(1)
+            raise ValueError("Invalid arguments: either query [-Q] or input file [-i] may be "
+                             "specified.")
 
     def __del__(self):
         # Shut-down sqltoolsservice
         if self.sqltoolsclient:
             self.sqltoolsclient.shutdown()
 
-    def write_to_file(self, pattern, **_):
-        if not pattern:
-            self.output_file = None
-            message = 'File output disabled'
-            return [(None, None, None, message, '', True)]
-        filename = os.path.abspath(os.path.expanduser(pattern))
-        if not os.path.isfile(filename):
-            try:
-                open(filename, 'w').close()
-            except IOError as e:
-                self.output_file = None
-                message = str(e) + '\nFile output disabled'
-                return [(None, None, None, message, '', False)]
-        self.output_file = filename
-        message = 'Writing to file "%s"' % self.output_file
-        return [(None, None, None, message, '', True)]
+    # TODO: possibly use at a later date for expanded output file functionality
+    # def write_to_file(self, pattern, **_):
+    #     if not pattern:
+    #         self.output_file = None
+    #         message = 'File output disabled'
+    #         return [(None, None, None, message, '', True)]
+    #     filename = os.path.abspath(os.path.expanduser(pattern))
+    #     if not os.path.isfile(filename):
+    #         try:
+    #             open(filename, 'w').close()
+    #         except IOError as e:
+    #             self.output_file = None
+    #             message = str(e) + '\nFile output disabled'
+    #             return [(None, None, None, message, '', False)]
+    #     self.output_file = filename
+    #     message = 'Writing to file "%s"' % self.output_file
+    #     return [(None, None, None, message, '', True)]
 
     def initialize_logging(self):
         log_file = self.config['main']['log_file']
@@ -390,15 +389,13 @@ def run(self):
 
         # raise error if interactive mode is set to false here
         if not self.interactive_mode:
-            raise ValueError("'run' must be used in interactive mode! Please set \
-                             interactive_mode to True.")
+            raise ValueError("Invalid arguments: 'run' must be used in interactive mode! Please set "
+                             "interactive_mode to True.")
 
         # exit and return error if user enters interactive mode with -o argument enabled
         if self.output_file:
-            click.secho("Invalid arguments: -o must be used with interactive mode set to false.",
-                        err=True, fg='red')
-            self.shutdown()
-            sys.exit(1)
+            raise ValueError("Invalid arguments: -o must be used with interactive mode set to "
+                             "false.")
 
         history_file = self.config['main']['history_file']
         if history_file == 'default':

tests/mssqltestutils.py

@@ -9,13 +9,22 @@
 from utility import random_str
 
 
+_BASELINE_DIR = os.path.dirname(os.path.abspath(__file__))
+
+# test queries mapped to files
+test_queries = [
+    ("SELECT 1", 'small.txt'),
+    ("SELECT 1; SELECT 2;", 'multiple.txt'),
+    ("SELECT %s" % ('x' * 250), 'col_too_wide.txt'),
+    ("SELECT REPLICATE(CAST('X,' AS VARCHAR(MAX)), 1024)", 'col_wide.txt')
+]
+
 def create_mssql_cli(**non_default_options):
     mssqlcli_options = create_mssql_cli_options(**non_default_options)
     mssql_cli = MssqlCli(mssqlcli_options)
 
     return mssql_cli
 
-
 def create_mssql_cli_client(options=None, owner_uri=None, connect=True, sql_tools_client=None,
                             **additional_params):
     """
@@ -43,7 +52,6 @@ def create_mssql_cli_client(options=None, owner_uri=None, connect=True, sql_tool
         print('Connection failed')
         raise e
 
-
 def create_mssql_cli_options(**nondefault_options):
 
     parser = create_parser()
@@ -62,7 +70,6 @@ def create_mssql_cli_options(**nondefault_options):
 
     return default_mssql_cli_options
 
-
 def shutdown(connection):
     connection.shutdown()
 
@@ -97,7 +104,6 @@ def create_test_db():
     clean_up_test_db(test_db_name)
     raise AssertionError("DB creation failed.")
 
-
 def clean_up_test_db(test_db_name):
     client = create_mssql_cli_client()
     query = u"DROP DATABASE {0};".format(test_db_name)
@@ -107,3 +113,18 @@ def clean_up_test_db(test_db_name):
             success = False
     shutdown(client)
     return success
+
+def get_file_contents(file_path):
+    """ Get expected result from file. """
+    try:
+        with open(file_path, 'r') as f:
+            # remove string literals (needed in python2) and newlines
+            return f.read().replace('\r', '').strip()
+    except OSError as e:
+        raise e
+
+def get_io_paths(test_file_suffix):
+    """ Returns tuple of file paths for the input and output of a test. """
+    i = os.path.join(_BASELINE_DIR, 'test_query_inputs', 'input_%s' % test_file_suffix)
+    o = os.path.join(_BASELINE_DIR, 'test_query_baseline', 'baseline_%s' % test_file_suffix)
+    return (i, o)

tests/test_interactive_mode.py

@@ -0,0 +1,75 @@
+import pytest
+from mssqltestutils import (
+    create_mssql_cli,
+    shutdown,
+    test_queries,
+    get_file_contents,
+    get_io_paths
+)
+
+class TestInteractiveModeQueries:
+    @staticmethod
+    @pytest.fixture(scope='class')
+    def mssqlcli():
+        """
+        Pytest fixture which returns interactive mssql-cli instance
+        and cleans up on teardown.
+        """
+        mssqlcli = create_mssql_cli(interactive_mode=True)
+        yield mssqlcli
+        shutdown(mssqlcli)
+
+    @staticmethod
+    @pytest.mark.parametrize("query_str, test_file", test_queries)
+    @pytest.mark.timeout(60)
+    def test_query(query_str, test_file, mssqlcli):
+        _, file_baseline = get_io_paths(test_file)
+        output_baseline = get_file_contents(file_baseline)
+        output_query = '\n'.join(mssqlcli.execute_query(query_str)).replace('\r', '')
+        assert output_query == output_baseline
+
+class TestInteractiveModeInvalidRuns:
+    @pytest.mark.timeout(60)
+    def test_noninteractive_run(self):
+        '''
+        Test that calling run throws an exception only when interactive_mode is false
+        '''
+        self.invalid_run(interactive_mode=False)
+
+    def test_interactive_output(self):
+        '''
+        Test run with interactive mode enabled with output file, which should return ValueError
+        '''
+        self.invalid_run(output_file='will-fail.txt')
+
+    @staticmethod
+    def test_output_with_interactive_change():
+        '''
+        Fails on run after interactive mode has been toggled
+        '''
+        mssqlcli = create_mssql_cli(interactive_mode=False, output_file='will-fail-eventually.txt')
+        mssqlcli.interactive_mode = True
+        try:
+            mssqlcli.run()
+            assert False
+        except ValueError:
+            assert True
+        finally:
+            shutdown(mssqlcli)
+
+    @staticmethod
+    @pytest.mark.timeout(60)
+    def invalid_run(**options):
+        '''
+        Tests mssql-cli runs with invalid combination of properities set
+        '''
+        mssqlcli = None
+        try:
+            mssqlcli = create_mssql_cli(**options)
+            mssqlcli.run()
+            assert False
+        except ValueError:
+            assert True
+        finally:
+            if mssqlcli is not None:
+                shutdown(mssqlcli)

tests/test_noninteractive_mode.py

@@ -6,10 +6,14 @@
 import pytest
 from mssqltestutils import (
     create_mssql_cli,
-    random_str
+    random_str,
+    shutdown,
+    test_queries,
+    get_file_contents,
+    get_io_paths,
+    _BASELINE_DIR
 )
 
-_BASELINE_DIR = os.path.dirname(os.path.abspath(__file__))
 
 class TestNonInteractiveResults:
     """
@@ -24,20 +28,14 @@ def tmp_filepath():
         yield fp
         os.remove(fp)
 
-    testdata = [
-        ("-Q \"SELECT 1\"", 'small.txt'),
-        ("-Q \"SELECT 1; SELECT 2;\"", 'multiple.txt'),
-        ("-Q \"SELECT %s\"" % ('x' * 250), 'col_too_wide.txt'),
-        ("-Q \"SELECT REPLICATE(CAST('X,' AS VARCHAR(MAX)), 1024)\"", 'col_wide.txt')
-    ]
-
-    @pytest.mark.parametrize("query_str, test_file", testdata)
+    @pytest.mark.parametrize("query_str, test_file", test_queries)
     @pytest.mark.timeout(60)
     def test_query(self, query_str, test_file):
         """ Tests query outputs to command-line, ensuring -Q and -i produce
         the same results. """
-        file_input, file_baseline = self.input_output_paths(test_file)
-        output_baseline = self.get_file_contents(file_baseline)
+        file_input, file_baseline = get_io_paths(test_file)
+        output_baseline = get_file_contents(file_baseline)
+        query_str = '-Q "{}"'.format(query_str)     # append -Q for non-interactive call
 
         # test with -Q
         output_query_for_Q = self.execute_query_via_subprocess(query_str)
@@ -47,12 +45,13 @@ def test_query(self, query_str, test_file):
         output_query_for_i = self.execute_query_via_subprocess("-i %s" % file_input)
         assert output_query_for_i == output_baseline
 
-    @pytest.mark.parametrize("query_str, test_file", testdata)
+    @pytest.mark.parametrize("query_str, test_file", test_queries)
     @pytest.mark.timeout(60)
     def test_output_file(self, query_str, test_file, tmp_filepath):
         """ Tests -o (and ensures file overwrite works) """
-        file_input, file_baseline = self.input_output_paths(test_file)
-        output_baseline = self.get_file_contents(file_baseline)
+        file_input, file_baseline = get_io_paths(test_file)
+        output_baseline = get_file_contents(file_baseline)
+        query_str = '-Q "{}"'.format(query_str)     # append -Q for non-interactive call
 
         # test with -Q
         output_query_for_Q = self.execute_query_via_subprocess(query_str, output_file=tmp_filepath)
@@ -63,35 +62,23 @@ def test_output_file(self, query_str, test_file, tmp_filepath):
                                                                output_file=tmp_filepath)
         assert output_query_for_i == output_baseline
 
+    @staticmethod
     @pytest.mark.timeout(60)
-    def test_long_query(self, tmp_filepath):
+    def test_long_query(tmp_filepath):
         """ Output large query using Python class instance. """
         query_str = "SELECT * FROM STRING_SPLIT(REPLICATE(CAST('X,' AS VARCHAR(MAX)), 1024), ',')"
         try:
             mssqlcli = create_mssql_cli(interactive_mode=False, output_file=tmp_filepath)
             output_query = '\n'.join(mssqlcli.execute_query(query_str))
-            file_baseline = self.input_output_paths('big.txt')[1]
-            output_baseline = self.get_file_contents(file_baseline)
+            file_baseline = get_io_paths('big.txt')[1]
+            output_baseline = get_file_contents(file_baseline)
             assert output_query == output_baseline
 
             # test output to file
-            output_query_from_file = self.get_file_contents(tmp_filepath)
+            output_query_from_file = get_file_contents(tmp_filepath)
             assert output_query_from_file == output_baseline
         finally:
-            mssqlcli.shutdown()
-
-    @staticmethod
-    @pytest.mark.timeout(60)
-    def test_noninteractive_run():
-        """ Test that calling run throws an exception only when interactive_mode is false """
-        mssqlcli = create_mssql_cli(interactive_mode=False)
-        try:
-            mssqlcli.run()
-            assert False
-        except ValueError:
-            assert True
-        finally:
-            mssqlcli.shutdown()
+            shutdown(mssqlcli)
 
     @classmethod
     @pytest.mark.timeout(60)
@@ -100,13 +87,6 @@ def test_Q_with_i_run(cls):
         output = cls.execute_query_via_subprocess("-Q 'select 1' -i 'this_breaks.txt'")
         assert output == "Invalid arguments: either -Q or -i may be specified."
 
-    @staticmethod
-    def input_output_paths(test_file_suffix):
-        """ Returns tuple of file paths for the input an output of a test. """
-        i = os.path.join(_BASELINE_DIR, 'test_query_inputs', 'input_%s' % test_file_suffix)
-        o = os.path.join(_BASELINE_DIR, 'test_query_baseline', 'baseline_%s' % test_file_suffix)
-        return (i, o)
-
     @classmethod
     def execute_query_via_subprocess(cls, query_str, output_file=None):
         """ Helper method for running a query. """
@@ -122,19 +102,9 @@ def execute_query_via_subprocess(cls, query_str, output_file=None):
 
         if output_file:
             # get file contents if we used -o
-            return cls.get_file_contents(output_file)
+            return get_file_contents(output_file)
         return output.decode("utf-8").replace('\r', '').strip()
 
-    @staticmethod
-    def get_file_contents(file_path):
-        """ Get expected result from file. """
-        try:
-            with open(file_path, 'r') as f:
-                # remove string literals (needed in python2) and newlines
-                return f.read().replace('\r', '').strip()
-        except OSError as e:
-            raise e
-
 
 class TestNonInteractiveShutdownQuery:
     """
@@ -160,7 +130,7 @@ def test_shutdown_after_query(query_str, mssqlcli):
         try:
             mssqlcli.execute_query(query_str)
         finally:
-            mssqlcli.shutdown()
+            shutdown(mssqlcli)
             assert mssqlcli.mssqlcliclient_main.sql_tools_client.\
                 tools_service_process.poll() is not None
 
@@ -192,6 +162,6 @@ def test_shutdown_after_query(query_str, mssqlcli):
         try:
             mssqlcli.execute_query(query_str)
         finally:
-            mssqlcli.shutdown()
+            shutdown(mssqlcli)
             assert mssqlcli.mssqlcliclient_main.sql_tools_client.\
                 tools_service_process.poll() is not None