Python Bug Fixes (Azure#928)

* Fix serializing lists into paths * Fix for streaming bug * Generated Python config type checking * Regenerated tests * fixed streaming bug * regenerated samples * Fixed runtime tests * Test server debuging * removed stream connection closing * ping build server
jianghaolu · Apr 15, 2016 · 29dc46d · 29dc46d
1 parent 14b4e56
commit 29dc46d
Show file tree

Hide file tree

Showing 29 changed files with 181 additions and 99 deletions.
diff --git a/...nceTests/AzureBodyDuration/autorestdurationtestservice/auto_rest_duration_test_service.py b/...nceTests/AzureBodyDuration/autorestdurationtestservice/auto_rest_duration_test_service.py
@@ -43,7 +43,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'https://localhost'
 

diff --git a/...rouping/autorestparametergroupingtestservice/auto_rest_parameter_grouping_test_service.py b/...rouping/autorestparametergroupingtestservice/auto_rest_parameter_grouping_test_service.py
@@ -43,7 +43,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'https://localhost'
 

diff --git a/...anceTests/AzureReport/autorestreportserviceforazure/auto_rest_report_service_for_azure.py b/...anceTests/AzureReport/autorestreportserviceforazure/auto_rest_report_service_for_azure.py
@@ -44,7 +44,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...ource/autorestresourceflatteningtestservice/auto_rest_resource_flattening_test_service.py b/...ource/autorestresourceflatteningtestservice/auto_rest_resource_flattening_test_service.py
@@ -44,7 +44,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...utorestazurespecialparameterstestclient/auto_rest_azure_special_parameters_test_client.py b/...utorestazurespecialparameterstestclient/auto_rest_azure_special_parameters_test_client.py
@@ -56,9 +56,15 @@ def __init__(
             self, credentials, subscription_id, api_version='2015-07-01-preview', accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
         if subscription_id is None:
-            raise ValueError('subscription_id must not be None.')
+            raise ValueError("Parameter 'subscription_id' must not be None.")
+        if not isinstance(subscription_id, str):
+            raise TypeError("Parameter 'subscription_id' must be str.")
+        if api_version is not None and not isinstance(api_version, str):
+            raise TypeError("Optional parameter 'api_version' must be str.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...omBaseUri/autorestparameterizedhosttestclient/auto_rest_parameterized_host_test_client.py b/...omBaseUri/autorestparameterizedhosttestclient/auto_rest_parameterized_host_test_client.py
@@ -45,9 +45,13 @@ def __init__(
             self, credentials, host, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
         if host is None:
-            raise ValueError('host must not be None.')
+            raise ValueError("Parameter 'host' must not be None.")
+        if not isinstance(host, str):
+            raise TypeError("Parameter 'host' must be str.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         base_url = 'http://{accountName}{host}'
 
         super(AutoRestParameterizedHostTestClientConfiguration, self).__init__(base_url, filepath)

diff --git a/...ests/Expected/AcceptanceTests/Head/autorestheadtestservice/auto_rest_head_test_service.py b/...ests/Expected/AcceptanceTests/Head/autorestheadtestservice/auto_rest_head_test_service.py
@@ -42,7 +42,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/.../HeadExceptions/autorestheadexceptiontestservice/auto_rest_head_exception_test_service.py b/.../HeadExceptions/autorestheadexceptiontestservice/auto_rest_head_exception_test_service.py
@@ -42,7 +42,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/.../autorestlongrunningoperationtestservice/auto_rest_long_running_operation_test_service.py b/.../autorestlongrunningoperationtestservice/auto_rest_long_running_operation_test_service.py
@@ -46,7 +46,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...xpected/AcceptanceTests/Paging/autorestpagingtestservice/auto_rest_paging_test_service.py b/...xpected/AcceptanceTests/Paging/autorestpagingtestservice/auto_rest_paging_test_service.py
@@ -43,7 +43,9 @@ def __init__(
             self, credentials, accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...eptanceTests/StorageManagementClient/storagemanagementclient/storage_management_client.py b/...eptanceTests/StorageManagementClient/storagemanagementclient/storage_management_client.py
@@ -50,9 +50,15 @@ def __init__(
             self, credentials, subscription_id, api_version='2015-05-01-preview', accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
         if subscription_id is None:
-            raise ValueError('subscription_id must not be None.')
+            raise ValueError("Parameter 'subscription_id' must not be None.")
+        if not isinstance(subscription_id, str):
+            raise TypeError("Parameter 'subscription_id' must be str.")
+        if api_version is not None and not isinstance(api_version, str):
+            raise TypeError("Optional parameter 'api_version' must be str.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'https://management.azure.com'
 

diff --git a/...cceptanceTests/SubscriptionIdApiVersion/microsoftazuretesturl/microsoft_azure_test_url.py b/...cceptanceTests/SubscriptionIdApiVersion/microsoftazuretesturl/microsoft_azure_test_url.py
@@ -47,9 +47,15 @@ def __init__(
             self, credentials, subscription_id, api_version='2014-04-01-preview', accept_language='en-US', long_running_operation_retry_timeout=30, generate_client_request_id=True, base_url=None, filepath=None):
 
         if credentials is None:
-            raise ValueError('credentials must not be None.')
+            raise ValueError("Parameter 'credentials' must not be None.")
         if subscription_id is None:
-            raise ValueError('subscription_id must not be None.')
+            raise ValueError("Parameter 'subscription_id' must not be None.")
+        if not isinstance(subscription_id, str):
+            raise TypeError("Parameter 'subscription_id' must be str.")
+        if api_version is not None and not isinstance(api_version, str):
+            raise TypeError("Optional parameter 'api_version' must be str.")
+        if accept_language is not None and not isinstance(accept_language, str):
+            raise TypeError("Optional parameter 'accept_language' must be str.")
         if not base_url:
             base_url = 'https://management.azure.com/'
 

diff --git a/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/file_tests.py b/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/file_tests.py
@@ -53,24 +53,21 @@ class FileTests(unittest.TestCase):
     def test_files(self):
 
         config = AutoRestSwaggerBATFileServiceConfiguration(base_url="http://localhost:3000")
-        config.log_level = log_level
         config.connection.data_block_size = 1000
         client = AutoRestSwaggerBATFileService(config)
 
-        def test_callback(data, response, progress = [0], is_response_streamed=None):
+        def test_callback(data, response, progress=[0]):
             self.assertTrue(len(data) > 0)
-            if not is_response_streamed:
-                self.assertFalse(response._content_consumed)
+            self.assertIsNotNone(response)
+            self.assertFalse(response._content_consumed)
+            total = float(response.headers.get('Content-Length', 0))
+            if total:
                 progress[0] += len(data)
-                total = float(response.headers['Content-Length'])
                 print("Downloading... {}%".format(int(progress[0]*100/total)))
-            self.assertIsNotNone(response)
 
         file_length = 0
         with io.BytesIO() as file_handle:
-
-            stream = client.files.get_file(callback=lambda x, response, progress=[0] :
-                                           test_callback(x, response, progress, False))
+            stream = client.files.get_file(callback=test_callback)
 
             for data in stream:
                 file_length += len(data)
@@ -86,46 +83,41 @@ def test_callback(data, response, progress = [0], is_response_streamed=None):
                 sample_data = hash(data.read())
             self.assertEqual(sample_data, hash(file_handle.getvalue()))
 
+        config.connection.data_block_size = 4096
         file_length = 0
         with io.BytesIO() as file_handle:
-
-            stream = client.files.get_empty_file(callback=lambda x, response, progress=[0] :
-                                                 test_callback(x, response, progress, False))
+            stream = client.files.get_empty_file(callback=test_callback)
 
             for data in stream:
                 file_length += len(data)
                 file_handle.write(data)
 
             self.assertEqual(file_length, 0)
 
-        #file_length = 0
-        #stream = client.files.get_file_large(callback=lambda x, response, progress=[0] :
-        #                                     test_callback(x, response, progress, True))
-        #for data in stream:
-        #    file_length += len(data)
+        file_length = 0
+        stream = client.files.get_file_large(callback=test_callback)
+        for data in stream:
+            file_length += len(data)
 
-        #self.assertEqual(file_length, 3000 * 1024 * 1024)
+        self.assertEqual(file_length, 3000 * 1024 * 1024)
 
     def test_files_raw(self):
 
-        def test_callback(data, response, progress = [0], is_response_streamed=None):
+        def test_callback(data, response, progress=[0]):
             self.assertTrue(len(data) > 0)
-            if not is_response_streamed:
-                self.assertFalse(response._content_consumed)
+            self.assertIsNotNone(response)
+            self.assertFalse(response._content_consumed)
+            total = float(response.headers.get('Content-Length', 0))
+            if total:
                 progress[0] += len(data)
-                total = float(response.headers['Content-Length'])
                 print("Downloading... {}%".format(int(progress[0]*100/total)))
-            self.assertIsNotNone(response)
 
         config = AutoRestSwaggerBATFileServiceConfiguration(base_url="http://localhost:3000")
-        config.log_level = log_level
         client = AutoRestSwaggerBATFileService(config)
 
         file_length = 0
         with io.BytesIO() as file_handle:
-
-            response = client.files.get_file(raw=True, callback=lambda x, response, progress=[0] :
-                                             test_callback(x, response, progress, False))
+            response = client.files.get_file(raw=True, callback=test_callback)
             stream = response.output
 
             for data in stream:
@@ -144,9 +136,7 @@ def test_callback(data, response, progress = [0], is_response_streamed=None):
 
         file_length = 0
         with io.BytesIO() as file_handle:
-
-            response = client.files.get_empty_file(raw=True, callback=lambda x, response, progress=[0] :
-                                                   test_callback(x, response, progress, False))
+            response = client.files.get_empty_file(raw=True, callback=test_callback)
             stream = response.output
 
             for data in stream:
@@ -155,16 +145,5 @@ def test_callback(data, response, progress = [0], is_response_streamed=None):
 
             self.assertEqual(file_length, 0)
 
-        #file_length = 0
-        #response = client.files.get_file_large(raw=True, callback=lambda x, response, progress=[0] :
-        #                                     test_callback(x, response, progress, True))
-
-        #stream = response.output
-
-        #for data in stream:
-        #    file_length += len(data)
-
-        #self.assertEqual(file_length, 3000 * 1024 * 1024)
-
 if __name__ == '__main__':
     unittest.main()
diff --git a/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/form_data_tests.py b/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/form_data_tests.py
@@ -174,7 +174,6 @@ def test_callback(data, response, progress = [0]):
         with open(self.dummy_file, 'rb') as upload_data:
             resp = client.formdata.upload_file_via_body(upload_data, callback=test_callback)
             for r in resp:
-                print(r)
                 result.write(r)
             self.assertEqual(result.getvalue().decode(), "Test file")
 

diff --git a/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/url_tests.py b/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/url_tests.py
@@ -90,8 +90,9 @@ def test_url_path(self):
         self.client.paths.get_negative_ten_billion(-10000000000)
         self.client.paths.get_ten_billion(10000000000)
         self.client.paths.string_empty("")
-        #test_array = ["ArrayPath1", r"begin!*'();:@ &=+$,/?#[]end", None, ""]
-        #self.client.paths.array_csv_in_path(test_array)
+
+        test_array = ["ArrayPath1", r"begin!*'();:@ &=+$,/?#[]end", None, ""]
+        self.client.paths.array_csv_in_path(test_array)
 
         with self.assertRaises(ValidationError):
             self.client.paths.string_null(None)

diff --git a/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/zzz_tests.py b/AutoRest/Generators/Python/Python.Tests/AcceptanceTests/zzz_tests.py
@@ -54,33 +54,38 @@ def test_ensure_coverage(self):
         client = AutoRestReportService(config)
         report = client.get_report()
 
-        # These will not be supported in Python
-        report['getIntegerOverflow']=1
-        report['getIntegerUnderflow']=1
-        report['getLongOverflow']=1
-        report['getLongUnderflow']=1
-        report['getDateInvalid']=1
-        report['getDictionaryNullkey']=1
-        report['HttpRedirect300Get']=1
-
-        # TODO: Support large file streams
-        report['FileStreamVeryLarge']=1
+        not_supported = {
+            'getIntegerOverflow': 1,
+            'getIntegerUnderflow': 1,
+            'getLongOverflow': 1,
+            'getLongUnderflow': 1,
+            'getDateInvalid': 1,
+            'getDictionaryNullkey': 1,
+            'HttpRedirect300Get': 1,
+        }
 
         # TODO: Support ignore readonly property in http put
-        report['putComplexReadOnlyPropertyValid']=1
+        missing_features_or_bugs = {
+            'putComplexReadOnlyPropertyValid': 1,
+        }
 
-        skipped = [k for k, v in report.items() if v == 0]
-        manually_marked_successful = [k for k, v in report.items() if v == 2]
-        for s in manually_marked_successful:
-            print("SKIPPED {0}".format(s))
+        report.update(not_supported)
+        report.update(missing_features_or_bugs)
+        failed = [k for k, v in report.items() if v == 0]
 
-        for s in skipped:
+        for s in not_supported.keys():
+            print("IGNORING {0}".format(s))
+
+        for s in missing_features_or_bugs.keys():
+            print("PENDING {0}".format(s))
+
+        for s in failed:
             print("FAILED TO EXECUTE {0}".format(s))
 
         totalTests = len(report)
-        print ("The test coverage is {0}/{1}.".format(totalTests - len(skipped), totalTests))
+        print ("The test coverage is {0}/{1}.".format(totalTests - len(failed), totalTests))
 
-        self.assertEqual(0, len(skipped))
+        self.assertEqual(0, len(failed))
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/.../AcceptanceTests/BodyComplex/autorestcomplextestservice/auto_rest_complex_test_service.py b/.../AcceptanceTests/BodyComplex/autorestcomplextestservice/auto_rest_complex_test_service.py
@@ -38,7 +38,9 @@ def __init__(
             self, api_version, base_url=None, filepath=None):
 
         if api_version is None:
-            raise ValueError('api_version must not be None.')
+            raise ValueError("Parameter 'api_version' must not be None.")
+        if not isinstance(api_version, str):
+            raise TypeError("Parameter 'api_version' must be str.")
         if not base_url:
             base_url = 'http://localhost'
 

diff --git a/...omBaseUri/autorestparameterizedhosttestclient/auto_rest_parameterized_host_test_client.py b/...omBaseUri/autorestparameterizedhosttestclient/auto_rest_parameterized_host_test_client.py
@@ -31,7 +31,9 @@ def __init__(
             self, host, filepath=None):
 
         if host is None:
-            raise ValueError('host must not be None.')
+            raise ValueError("Parameter 'host' must not be None.")
+        if not isinstance(host, str):
+            raise TypeError("Parameter 'host' must be str.")
         base_url = 'http://{accountName}{host}'
 
         super(AutoRestParameterizedHostTestClientConfiguration, self).__init__(base_url, filepath)

diff --git a/...dOptional/autorestrequiredoptionaltestservice/auto_rest_required_optional_test_service.py b/...dOptional/autorestrequiredoptionaltestservice/auto_rest_required_optional_test_service.py
@@ -36,9 +36,13 @@ def __init__(
             self, required_global_path, required_global_query, optional_global_query=None, base_url=None, filepath=None):
 
         if required_global_path is None:
-            raise ValueError('required_global_path must not be None.')
+            raise ValueError("Parameter 'required_global_path' must not be None.")
+        if not isinstance(required_global_path, str):
+            raise TypeError("Parameter 'required_global_path' must be str.")
         if required_global_query is None:
-            raise ValueError('required_global_query must not be None.')
+            raise ValueError("Parameter 'required_global_query' must not be None.")
+        if not isinstance(required_global_query, str):
+            raise TypeError("Parameter 'required_global_query' must be str.")
         if not base_url:
             base_url = 'http://localhost'