From 5b9a16eff444e42ef13dbf9640da708e881f16d9 Mon Sep 17 00:00:00 2001 From: "Varad Meru [gmail]" Date: Sun, 4 Oct 2020 01:06:58 -0700 Subject: [PATCH] Fixing pydoc comments and flaky tests * Fixing ''' to """ string for PyDocs * Adding critical logs to hit that code path * Marking some tests as flaky to allow it to be rerun. * Adding some sleep when starting the test host/worker to allow it to start on mac. * Bump grpcio from 1.26.0 to 1.32.0. --- azure_functions_worker/dispatcher.py | 14 ++++-- azure_functions_worker/testutils.py | 4 +- python/prodV2/worker.py | 8 ++-- python/prodV3/worker.config.json | 4 +- python/prodV3/worker.py | 10 ++-- setup.py | 7 +-- .../endtoend/test_eventhub_batch_functions.py | 4 +- tests/endtoend/test_eventhub_functions.py | 4 +- .../http_functions/debug_logging/main.py | 1 + .../http_functions/debug_user_logging/main.py | 1 + .../http_functions/sync_logging/main.py | 1 + .../load_functions/pytest/__init__.py | 4 +- tests/unittests/test_dispatcher.py | 28 ++++------- tests/unittests/test_http_functions.py | 5 ++ tests/unittests/test_loader.py | 47 ++++++++++--------- tests/unittests/test_mock_http_functions.py | 3 +- tests/unittests/test_rpc_messages.py | 21 ++++----- 17 files changed, 90 insertions(+), 76 deletions(-) diff --git a/azure_functions_worker/dispatcher.py b/azure_functions_worker/dispatcher.py index 0a7852e2b..fc1850cca 100644 --- a/azure_functions_worker/dispatcher.py +++ b/azure_functions_worker/dispatcher.py @@ -37,6 +37,14 @@ _TRUE = "true" +"""In Python 3.6, the current_task method was in the Task class, but got moved +out in 3.7+ and fully removed in 3.9. Thus, to support 3.6 and 3.9 together, we +need to switch the implementation of current_task for 3.6. +""" +_CURRENT_TASK = asyncio.Task.current_task \ + if (sys.version_info[0] == 3 and sys.version_info[1] == 6) \ + else asyncio.current_task + class DispatcherMeta(type): @@ -302,7 +310,7 @@ async def _handle__invocation_request(self, req): invoc_request.trace_context.attributes) # Set the current `invocation_id` to the current task so # that our logging handler can find it. - current_task = asyncio.Task.current_task(self._loop) + current_task = _CURRENT_TASK(self._loop) assert isinstance(current_task, ContextEnabledTask) current_task.set_azure_invocation_id(invocation_id) @@ -576,7 +584,7 @@ class ContextEnabledTask(asyncio.Task): def __init__(self, coro, loop): super().__init__(coro, loop=loop) - current_task = asyncio.Task.current_task(loop) + current_task = _CURRENT_TASK(loop) if current_task is not None: invocation_id = getattr( current_task, self.AZURE_INVOCATION_ID, None) @@ -590,7 +598,7 @@ def set_azure_invocation_id(self, invocation_id: str) -> None: def get_current_invocation_id() -> Optional[str]: loop = asyncio._get_running_loop() if loop is not None: - current_task = asyncio.Task.current_task(loop) + current_task = _CURRENT_TASK(loop) if current_task is not None: task_invocation_id = getattr(current_task, ContextEnabledTask.AZURE_INVOCATION_ID, diff --git a/azure_functions_worker/testutils.py b/azure_functions_worker/testutils.py index 870b3e0e4..5e3e77162 100644 --- a/azure_functions_worker/testutils.py +++ b/azure_functions_worker/testutils.py @@ -677,6 +677,7 @@ def start_webhost(*, script_dir=None, stdout=None): port = _find_open_port() proc = popen_webhost(stdout=stdout, stderr=subprocess.STDOUT, script_root=script_root, port=port) + time.sleep(3) # Giving host some time to start fully. addr = f'http://{LOCALHOST}:{port}' for _ in range(10): @@ -690,7 +691,8 @@ def start_webhost(*, script_dir=None, stdout=None): except requests.exceptions.ConnectionError: pass - time.sleep(1) + time.sleep(2) + else: proc.terminate() try: diff --git a/python/prodV2/worker.py b/python/prodV2/worker.py index a57848809..3b6b90882 100644 --- a/python/prodV2/worker.py +++ b/python/prodV2/worker.py @@ -17,20 +17,20 @@ def is_azure_environment(): - '''Check if the function app is running on the cloud''' + """Check if the function app is running on the cloud""" return (AZURE_CONTAINER_NAME in os.environ or AZURE_WEBSITE_INSTANCE_ID in os.environ) def add_script_root_to_sys_path(): - '''Append function project root to module finding sys.path''' + """Append function project root to module finding sys.path""" functions_script_root = os.getenv(AZURE_WEBJOBS_SCRIPT_ROOT) if functions_script_root is not None: sys.path.append(functions_script_root) def determine_user_pkg_paths(): - '''This finds the user packages when function apps are running on the cloud + """This finds the user packages when function apps are running on the cloud For Python 3.6 app, the third-party packages can live in any of the paths: /home/site/wwwroot/.python_packages/lib/site-packages @@ -39,7 +39,7 @@ def determine_user_pkg_paths(): For Python 3.7, we only accept: /home/site/wwwroot/.python_packages/lib/site-packages - ''' + """ minor_version = sys.version_info[1] home = Path.home() diff --git a/python/prodV3/worker.config.json b/python/prodV3/worker.config.json index da446a868..f935fd394 100644 --- a/python/prodV3/worker.config.json +++ b/python/prodV3/worker.config.json @@ -1,9 +1,9 @@ { "description":{ "language":"python", - "defaultRuntimeVersion":"3.6", + "defaultRuntimeVersion":"3.8", "supportedOperatingSystems":["LINUX", "OSX", "WINDOWS"], - "supportedRuntimeVersions":["3.6", "3.7", "3.8"], + "supportedRuntimeVersions":["3.6", "3.7", "3.8", "3.9"], "supportedArchitectures":["X64", "X86"], "extensions":[".py"], "defaultExecutablePath":"python", diff --git a/python/prodV3/worker.py b/python/prodV3/worker.py index 69644905d..a3006b6fa 100644 --- a/python/prodV3/worker.py +++ b/python/prodV3/worker.py @@ -17,20 +17,20 @@ def is_azure_environment(): - '''Check if the function app is running on the cloud''' + """Check if the function app is running on the cloud""" return (AZURE_CONTAINER_NAME in os.environ or AZURE_WEBSITE_INSTANCE_ID in os.environ) def add_script_root_to_sys_path(): - '''Append function project root to module finding sys.path''' + """Append function project root to module finding sys.path""" functions_script_root = os.getenv(AZURE_WEBJOBS_SCRIPT_ROOT) if functions_script_root is not None: sys.path.append(functions_script_root) def determine_user_pkg_paths(): - '''This finds the user packages when function apps are running on the cloud + """This finds the user packages when function apps are running on the cloud For Python 3.6 app, the third-party packages can live in any of the paths: /home/site/wwwroot/.python_packages/lib/site-packages @@ -39,7 +39,7 @@ def determine_user_pkg_paths(): For Python 3.7 and Python 3.8, we only accept: /home/site/wwwroot/.python_packages/lib/site-packages - ''' + """ minor_version = sys.version_info[1] home = Path.home() @@ -51,7 +51,7 @@ def determine_user_pkg_paths(): user_pkg_paths.append(os.path.join(venv_pkgs_path, PKGS_36)) user_pkg_paths.append(os.path.join(pkgs_path, PKGS_36)) user_pkg_paths.append(os.path.join(pkgs_path, PKGS)) - elif minor_version in (7, 8): + elif minor_version in (7, 8, 9): user_pkg_paths.append(os.path.join(pkgs_path, PKGS)) else: raise RuntimeError(f'Unsupported Python version: 3.{minor_version}') diff --git a/setup.py b/setup.py index bd98eb74e..c30734653 100644 --- a/setup.py +++ b/setup.py @@ -282,8 +282,8 @@ def run(self): 'azure_functions_worker.utils', 'azure_functions_worker._thirdparty'], install_requires=[ - 'grpcio~=1.26.0', - 'grpcio-tools~=1.26.0', + 'grpcio~=1.32.0', + 'grpcio-tools~=1.32.0', ], extras_require={ 'dev': [ @@ -299,7 +299,8 @@ def run(self): 'pytest-cov', 'pytest-xdist', 'pytest-randomly', - 'pytest-instafail' + 'pytest-instafail', + 'pytest-rerunfailures' ] }, include_package_data=True, diff --git a/tests/endtoend/test_eventhub_batch_functions.py b/tests/endtoend/test_eventhub_batch_functions.py index 97c9acbba..7ded59474 100644 --- a/tests/endtoend/test_eventhub_batch_functions.py +++ b/tests/endtoend/test_eventhub_batch_functions.py @@ -10,13 +10,13 @@ class TestEventHubFunctions(testutils.WebHostTestCase): - '''Test EventHub Trigger and Output Bindings (cardinality: many). + """Test EventHub Trigger and Output Bindings (cardinality: many). Each testcase consists of 3 part: 1. An eventhub_output_batch HTTP trigger for generating EventHub event 2. An eventhub_multiple EventHub trigger for converting event into blob 3. A get_eventhub_batch_triggered HTTP trigger for the event body - ''' + """ @classmethod def get_script_dir(cls): diff --git a/tests/endtoend/test_eventhub_functions.py b/tests/endtoend/test_eventhub_functions.py index 25bfdbe59..a47569484 100644 --- a/tests/endtoend/test_eventhub_functions.py +++ b/tests/endtoend/test_eventhub_functions.py @@ -9,13 +9,13 @@ class TestEventHubFunctions(testutils.WebHostTestCase): - '''Test EventHub Trigger and Output Bindings (cardinality: one). + """Test EventHub Trigger and Output Bindings (cardinality: one). Each testcase consists of 3 part: 1. An eventhub_output HTTP trigger for generating EventHub event 2. An actual eventhub_trigger EventHub trigger for storing event into blob 3. A get_eventhub_triggered HTTP trigger for retrieving event info blob - ''' + """ @classmethod def get_script_dir(cls): diff --git a/tests/unittests/http_functions/debug_logging/main.py b/tests/unittests/http_functions/debug_logging/main.py index be3e2d506..628896cbc 100644 --- a/tests/unittests/http_functions/debug_logging/main.py +++ b/tests/unittests/http_functions/debug_logging/main.py @@ -6,6 +6,7 @@ def main(req: azure.functions.HttpRequest): + logging.critical('logging critical', exc_info=True) logging.info('logging info', exc_info=True) logging.warning('logging warning', exc_info=True) logging.debug('logging debug', exc_info=True) diff --git a/tests/unittests/http_functions/debug_user_logging/main.py b/tests/unittests/http_functions/debug_user_logging/main.py index 6cf0465af..fc0b6d342 100644 --- a/tests/unittests/http_functions/debug_user_logging/main.py +++ b/tests/unittests/http_functions/debug_user_logging/main.py @@ -9,6 +9,7 @@ def main(req: azure.functions.HttpRequest): + logging.critical('logging critical', exc_info=True) logger.info('logging info', exc_info=True) logger.warning('logging warning', exc_info=True) logger.debug('logging debug', exc_info=True) diff --git a/tests/unittests/http_functions/sync_logging/main.py b/tests/unittests/http_functions/sync_logging/main.py index c87f8a404..71460bc26 100644 --- a/tests/unittests/http_functions/sync_logging/main.py +++ b/tests/unittests/http_functions/sync_logging/main.py @@ -14,5 +14,6 @@ def main(req: azure.functions.HttpRequest): 1 / 0 except ZeroDivisionError: logger.error('a gracefully handled error', exc_info=True) + logger.error('a gracefully handled critical error', exc_info=True) time.sleep(0.05) return 'OK-sync' diff --git a/tests/unittests/load_functions/pytest/__init__.py b/tests/unittests/load_functions/pytest/__init__.py index 4349b0878..ab7de2090 100644 --- a/tests/unittests/load_functions/pytest/__init__.py +++ b/tests/unittests/load_functions/pytest/__init__.py @@ -2,6 +2,6 @@ # Licensed under the MIT License. -'''This module pytest is provided inside customer's code, -used for checking module name collision''' +"""This module pytest is provided inside customer's code, +used for checking module name collision""" __version__ = 'from.customer.code' diff --git a/tests/unittests/test_dispatcher.py b/tests/unittests/test_dispatcher.py index 4bdf4c323..f3893d174 100644 --- a/tests/unittests/test_dispatcher.py +++ b/tests/unittests/test_dispatcher.py @@ -18,38 +18,32 @@ def tearDown(self): os.environ.update(self._pre_env) async def test_dispatcher_sync_threadpool_default_worker(self): - '''Test if the sync threadpool has maximum worker count set to 1 + """Test if the sync threadpool has maximum worker count set to 1 by default - ''' + """ ctrl = testutils.start_mockhost(script_root=self.dispatcher_funcs_dir) async with ctrl as host: await self._check_if_function_is_ok(host) - - # Ensure the dispatcher sync threadpool count is set to 1 self.assertEqual(ctrl._worker._sync_tp_max_workers, 1) async def test_dispatcher_sync_threadpool_set_worker(self): - '''Test if the sync threadpool maximum worker can be set - ''' + """Test if the sync threadpool maximum worker can be set + """ # Configure thread pool max worker os.environ.update({PYTHON_THREADPOOL_THREAD_COUNT: '5'}) ctrl = testutils.start_mockhost(script_root=self.dispatcher_funcs_dir) async with ctrl as host: await self._check_if_function_is_ok(host) - - # Ensure the dispatcher sync threadpool count is set to 1 self.assertEqual(ctrl._worker._sync_tp_max_workers, 5) @patch('azure_functions_worker.dispatcher.logger') - async def test_dispatcher_sync_threadpool_invalid_worker_count( - self, - mock_logger - ): - '''Test when sync threadpool maximum worker is set to an invalid value, + async def test_dispatcher_sync_threadpool_invalid_worker_count(self, + mock_logger): + """Test when sync threadpool maximum worker is set to an invalid value, the host should fallback to default value 1 - ''' + """ # Configure thread pool max worker to an invalid value os.environ.update({PYTHON_THREADPOOL_THREAD_COUNT: 'invalid'}) ctrl = testutils.start_mockhost(script_root=self.dispatcher_funcs_dir) @@ -64,10 +58,8 @@ async def test_dispatcher_sync_threadpool_invalid_worker_count( f'{PYTHON_THREADPOOL_THREAD_COUNT} must be an integer') @patch('azure_functions_worker.dispatcher.logger') - async def test_dispatcher_sync_threadpool_below_min_setting( - self, - mock_logger - ): + async def test_dispatcher_sync_threadpool_below_min_setting(self, + mock_logger): # Configure thread pool max worker to an invalid value os.environ.update({PYTHON_THREADPOOL_THREAD_COUNT: '0'}) ctrl = testutils.start_mockhost(script_root=self.dispatcher_funcs_dir) diff --git a/tests/unittests/test_http_functions.py b/tests/unittests/test_http_functions.py index 733e3f8ae..a98988f46 100644 --- a/tests/unittests/test_http_functions.py +++ b/tests/unittests/test_http_functions.py @@ -6,6 +6,8 @@ import typing import os +import pytest + from azure_functions_worker import testutils @@ -323,6 +325,7 @@ def check_log_import_module_troubleshooting_url(self, "Troubleshooting Guide: " "https://aka.ms/functions-modulenotfound", host_out) + @pytest.mark.flaky(reruns=3) def test_print_logging_no_flush(self): r = self.webhost.request('GET', 'print_logging?message=Secret42') self.assertEqual(r.status_code, 200) @@ -331,6 +334,7 @@ def test_print_logging_no_flush(self): def check_log_print_logging_no_flush(self, host_out: typing.List[str]): self.assertIn('Secret42', host_out) + @pytest.mark.flaky(reruns=3) def test_print_logging_with_flush(self): r = self.webhost.request('GET', 'print_logging?flush=true&message=Secret42') @@ -346,6 +350,7 @@ def test_print_to_console_stdout(self): self.assertEqual(r.status_code, 200) self.assertEqual(r.text, 'OK-print-logging') + @pytest.mark.flaky(reruns=3) def check_log_print_to_console_stdout(self, host_out: typing.List[str]): # System logs stdout should not exist in host_out self.assertNotIn('Secret42', host_out) diff --git a/tests/unittests/test_loader.py b/tests/unittests/test_loader.py index 7ab9c041b..77fb7bb2c 100644 --- a/tests/unittests/test_loader.py +++ b/tests/unittests/test_loader.py @@ -46,41 +46,46 @@ def test_loader_parentmodule(self): self.assertEqual(r.text, '__app__.parentmodule.module') def test_loader_absolute_thirdparty(self): - ''' Allow third-party package import from .python_packages - and worker_venv''' + """Allow third-party package import from .python_packages + and worker_venv + """ r = self.webhost.request('GET', 'absolute_thirdparty') self.assertEqual(r.status_code, 200) self.assertEqual(r.text, 'eh = azure.eventhub') def test_loader_prioritize_customer_module(self): - ''' When a module in customer code has the same name with a third-party - package, the worker should prioritize third-party package''' + """When a module in customer code has the same name with a third-party + package, the worker should prioritize third-party package + """ r = self.webhost.request('GET', 'name_collision') self.assertEqual(r.status_code, 200) self.assertRegex(r.text, r'pt.__version__ = \d+.\d+.\d+') def test_loader_fix_customer_module_with_app_import(self): - ''' When a module in customer code has the same name with a third-party + """When a module in customer code has the same name with a third-party package, if customer uses "import __app__." statement, - the worker should load customer package''' + the worker should load customer package + """ r = self.webhost.request('GET', 'name_collision_app_import') self.assertEqual(r.status_code, 200) self.assertEqual(r.text, 'pt.__version__ = from.customer.code') def test_loader_implicit_import(self): - ''' Since sys.path is now fixed with script root appended, - implicit import statement is now acceptable.''' + """Since sys.path is now fixed with script root appended, + implicit import statement is now acceptable. + """ r = self.webhost.request('GET', 'implicit_import') self.assertEqual(r.status_code, 200) self.assertEqual(r.text, 's_main = simple.main') def test_loader_module_not_found(self): - ''' If a module cannot be found, should throw an exception with - trouble shooting link https://aka.ms/functions-modulenotfound''' + """If a module cannot be found, should throw an exception with + trouble shooting link https://aka.ms/functions-modulenotfound + """ r = self.webhost.request('GET', 'module_not_found') self.assertEqual(r.status_code, 500) @@ -105,20 +110,20 @@ async def test_entry_point_plugin(self): # This test must be run in a subprocess so that # pkg_resources picks up the newly installed package. code = textwrap.dedent(''' - import asyncio - from azure_functions_worker import protos - from azure_functions_worker import testutils +import asyncio +from azure_functions_worker import protos +from azure_functions_worker import testutils - async def _runner(): - async with testutils.start_mockhost( - script_root='unittests/test-binding/functions') as host: - func_id, r = await host.load_function('foo') +async def _runner(): + async with testutils.start_mockhost( + script_root='unittests/test-binding/functions') as host: + func_id, r = await host.load_function('foo') - print(r.response.function_id == func_id) - print(r.response.result.status == protos.StatusResult.Success) + print(r.response.function_id == func_id) + print(r.response.result.status == protos.StatusResult.Success) - asyncio.get_event_loop().run_until_complete(_runner()) - ''') +asyncio.get_event_loop().run_until_complete(_runner()) +''') try: proc = await asyncio.create_subprocess_exec( diff --git a/tests/unittests/test_mock_http_functions.py b/tests/unittests/test_mock_http_functions.py index 929ccb7ae..e00ff32ac 100644 --- a/tests/unittests/test_mock_http_functions.py +++ b/tests/unittests/test_mock_http_functions.py @@ -24,7 +24,8 @@ async def test_call_sync_function_check_logs(self): user_logs = [line for line in r.logs if line.category == 'my function'] - self.assertEqual(len(user_logs), 1) + # 2 log statements added (critical and error) in sync_logging + self.assertEqual(len(user_logs), 2) log = user_logs[0] self.assertEqual(log.invocation_id, invoke_id) diff --git a/tests/unittests/test_rpc_messages.py b/tests/unittests/test_rpc_messages.py index 64f99ebac..1c4730669 100644 --- a/tests/unittests/test_rpc_messages.py +++ b/tests/unittests/test_rpc_messages.py @@ -3,8 +3,8 @@ import os import subprocess import sys -import typing import tempfile +import typing import unittest from azure_functions_worker import protos @@ -72,12 +72,9 @@ async def test_reload_env_message(self): await self._verify_environment_reloaded(test_env, test_cwd) def _verify_sys_path_import(self, result, expected_output): + path_import_script = os.path.join(testutils.UNIT_TESTS_ROOT, + 'path_import', 'test_path_import.sh') try: - path_import_script = os.path.join( - testutils.UNIT_TESTS_ROOT, - 'path_import', - 'test_path_import.sh') - subprocess.run(['chmod +x ' + path_import_script], shell=True) exported_path = ":".join(sys.path) @@ -87,6 +84,7 @@ def _verify_sys_path_import(self, result, expected_output): decoded_output = output.decode(sys.stdout.encoding).strip() self.assertTrue(expected_output in decoded_output) finally: + subprocess.run(['chmod -x ' + path_import_script], shell=True) self._reset_environ() @unittest.skipIf(sys.platform == 'win32', @@ -104,13 +102,11 @@ def test_successful_sys_path_import(self): 'This module was imported!') def _verify_azure_namespace_import(self, result, expected_output): + print(os.getcwd()) + path_import_script = os.path.join(testutils.UNIT_TESTS_ROOT, + 'azure_namespace_import', + 'test_azure_namespace_import.sh') try: - print(os.getcwd()) - path_import_script = os.path.join( - testutils.UNIT_TESTS_ROOT, - 'azure_namespace_import', - 'test_azure_namespace_import.sh') - subprocess.run(['chmod +x ' + path_import_script], shell=True) output = subprocess.check_output( @@ -119,6 +115,7 @@ def _verify_azure_namespace_import(self, result, expected_output): decoded_output = output.decode(sys.stdout.encoding).strip() self.assertTrue(expected_output in decoded_output) finally: + subprocess.run(['chmod -x ' + path_import_script], shell=True) self._reset_environ() @unittest.skipIf(sys.platform == 'win32',