Workaround lru_cache decorator on method garbage collection problem

.github/workflows/test.yml

@@ -10,51 +10,50 @@ jobs:
   test:
     runs-on: ${{ matrix.os }}
     timeout-minutes: 15
-    env:
-      PYTHON_COVERAGE_ENABLED: '3.8'
     strategy:
       matrix:
         os: ['ubuntu-latest']
         python: ['3.7', '3.8', '3.9']
         include:
           - os: 'macos-latest'
             python: '3.7'
+    env:
+      PYTEST_ADDOPTS: --cov --color=yes
 
     steps:
       - name: Checkout
-        uses: actions/checkout@v2
-        with:
-          fetch-depth: 2  # required by codecov
+        uses: actions/checkout@v3
 
       - name: Setup Python
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v3
         with:
           python-version: ${{ matrix.python }}
 
-      # NOTE: We install in both editable, and non-editable mode. The former is
-      # necessary to produce coverage reports with the correct numbers.
-      # https://github.com/cylc/cylc-uiserver/issues/179
+      # NOTE: Install cylc-uiserver in editable mode for correct coverage results
       - name: Install cylc-uiserver & dependencies
         run: |
           pip install git+https://github.com/cylc/cylc-flow@master
-          if [ "${{ matrix.python }}" == "${{ env.PYTHON_COVERAGE_ENABLED }}" ]; then
-            pip install -e .[all]
-          else
-            pip install .[all]
-          fi
+          pip install -e .[all]
 
+      - name: (Debug - pip list)
+        run: pip list --format=freeze
 
       - name: Style test
         run: flake8
 
-      - name: mypy
-        run: mypy --explicit-package-bases
+      - name: Type checking
+        run: mypy
 
       - name: Test
-        run: pytest --cov
+        run: pytest
 
-      - name: Coverage
-        if: ${{ success() &&  matrix.python == env.PYTHON_COVERAGE_ENABLED }}
+      - name: Coverage report
         run: |
           coverage xml --ignore-errors
-          bash <(curl -s https://codecov.io/bash)
+          coverage report
+
+      - name: Codecov upload
+        uses: codecov/codecov-action@v3
+        with:
+          name: '${{ matrix.os }} py-${{ matrix.python }}'
+          fail_ci_if_error: true

README.md

@@ -167,7 +167,7 @@ for all Cylc-specific configuration options.
 The Cylc UI Server is a
 [Jupyter Server](https://github.com/jupyter-server/jupyter_server) extension.
 Jupyter Server can run multiple extensions. To control the extensions that
-are run use the `ServerApp.jpserver_extensions` configuration, see the 
+are run use the `ServerApp.jpserver_extensions` configuration, see the
 [Jupyter Server configuration documentation](https://jupyter-server.readthedocs.io/en/latest/other/full-config.html#other-full-config).
 
 ### UI
@@ -215,6 +215,12 @@ Contributions welcome:
    c.CylcUIServer.ui_build_dir = '~/cylc-ui/dist'  # path to build
    ```
 
+Note about testing: unlike cylc-flow, cylc-uiserver uses the
+[pytest-tornasync](https://github.com/eukaryote/pytest-tornasync/) plugin
+instead of [pytest-asyncio](https://github.com/pytest-dev/pytest-asyncio).
+This means you should not decorate async test functions with
+`@pytest.mark.asyncio`.
+
 ## Copyright and Terms of Use
 
 Copyright (C) 2019-<span actions:bind='current-year'>2022</span> NIWA & British Crown (Met Office) & Contributors.

cylc/uiserver/authorise.py

@@ -107,6 +107,11 @@ def __init__(self, owner, owner_auth_conf, site_auth_conf, log) -> None:
         }
         self.owner_dict = self.build_owner_site_auth_conf()
 
+        # lru_cache this method - see flake8-bugbear B019
+        self.get_permitted_operations = lru_cache(maxsize=128)(
+            self._get_permitted_operations
+        )
+
     @staticmethod
     def expand_and_process_access_groups(permission_set: set) -> set:
         """Process a permission set.
@@ -225,8 +230,8 @@ def get_access_user_permissions_from_owner_conf(
         allowed_operations.discard("")
         return allowed_operations
 
-    @lru_cache(maxsize=128)  # noqa B019 TODO?
-    def get_permitted_operations(self, access_user: str):
+    # lru_cached - see __init__()
+    def _get_permitted_operations(self, access_user: str):
         """Return permitted operations for given access_user.
 
         Cached for efficiency.

cylc/uiserver/tests/conftest.py

@@ -72,19 +72,6 @@ def async_client():
     return AsyncClientFixture()
 
 
-@pytest.fixture
-def event_loop():
-    """This fixture defines the event loop used for each test."""
-    loop = asyncio.get_event_loop_policy().new_event_loop()
-    yield loop
-    # gracefully exit async generators
-    loop.run_until_complete(loop.shutdown_asyncgens())
-    # cancel any tasks still running in this event loop
-    for task in asyncio.all_tasks(loop):
-        task.cancel()
-    loop.close()
-
-
 @pytest.fixture
 def workflows_manager() -> WorkflowsManager:
     return WorkflowsManager(None, logging.getLogger('cylc'))
@@ -212,7 +199,7 @@ def authorisation_false(monkeypatch):
 
 
 @pytest.fixture
-def mock_authentication(monkeypatch):
+def mock_authentication(monkeypatch: pytest.MonkeyPatch):
 
     def _mock_authentication(user=None, server=None, none=False):
         ret = {
@@ -293,7 +280,6 @@ def patch_conf_files(monkeypatch):
     monkeypatch.setattr(
         'cylc.uiserver.app.CylcUIServer.config_file_paths', []
     )
-    yield
 
 
 @pytest.fixture

cylc/uiserver/tests/test_auth.py

@@ -18,7 +18,6 @@
 from functools import partial
 
 import pytest
-
 from tornado.httpclient import HTTPClientError
 
 

cylc/uiserver/tests/test_authorise.py

@@ -13,11 +13,11 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-# from logging import Logger as log
-from types import SimpleNamespace
 from jupyter_server.extension.application import ExtensionApp
+from types import SimpleNamespace
+from unittest.mock import Mock, patch
+
 import pytest
-from unittest.mock import patch
 
 from cylc.uiserver.authorise import (
     Authorization,
@@ -345,24 +345,22 @@ def test_get_op_name(mut_field_name, operation, expected_op_name):
         ),
     ],
 )
-@patch("cylc.uiserver.authorise.Authorization.get_permitted_operations")
 @patch("cylc.uiserver.authorise.get_groups")
 def test_is_permitted(
     mocked_get_groups,
-    mocked_get_permitted_operations,
     owner_name,
     user_name,
     get_permitted_operations_is_called,
     expected,
 ):
     mocked_get_groups.side_effect = [([""], []), ([""], [])]
-    mocked_get_permitted_operations.return_value = ["fake_operation"]
     auth_obj = Authorization(owner_name, FAKE_USER_CONF, FAKE_SITE_CONF, log)
+    auth_obj.get_permitted_operations = Mock(return_value=["fake_operation"])
     actual = auth_obj.is_permitted(
         access_user=user_name, operation="fake_operation"
     )
     if get_permitted_operations_is_called:
-        mocked_get_permitted_operations.assert_called_with(user_name)
+        auth_obj.get_permitted_operations.assert_called_with(user_name)
     assert actual == expected
 
 

cylc/uiserver/tests/test_config.py

@@ -19,8 +19,12 @@
 
 import pytest
 
+from cylc.flow.cfgspec.globalcfg import GlobalConfig
 from cylc.uiserver import __file__ as UIS_PKG
-from cylc.uiserver.config_util import get_conf_dir_hierarchy
+from cylc.uiserver.config_util import (
+    UISERVER_DIR,
+    get_conf_dir_hierarchy,
+)
 from cylc.uiserver.jupyterhub_config import load
 
 
@@ -30,7 +34,7 @@
 
 
 @pytest.fixture
-def clear_env(monkeypatch):
+def clear_env(monkeypatch: pytest.MonkeyPatch):
     for envvar in ('CYLC_SITE_CONF_PATH',):
         if envvar in os.environ:
             monkeypatch.delenv(envvar)
@@ -48,16 +52,21 @@ def capload(monkeypatch):
     return files
 
 
-def test_config(clear_env, capload, monkeypatch):
+def test_config(clear_env, capload: list, monkeypatch: pytest.MonkeyPatch):
     """It should load the system, site and user configs in that order."""
     monkeypatch.setattr(config_util, '__version__', '0')
+    # This constant was set before clear_env took effect:
+    monkeypatch.setattr(
+        'cylc.uiserver.jupyterhub_config.SITE_CONF_ROOT',
+        Path(GlobalConfig.DEFAULT_SITE_CONF_PATH, UISERVER_DIR)
+    )
     load()
     assert capload == [
         SYS_CONF,
-        (SITE_CONF/'jupyter_config.py'),
-        (SITE_CONF/'0/jupyter_config.py'),
-        (USER_CONF/'jupyter_config.py'),
-        (USER_CONF/'0/jupyter_config.py')
+        (SITE_CONF / 'jupyter_config.py'),
+        (SITE_CONF / '0/jupyter_config.py'),
+        (USER_CONF / 'jupyter_config.py'),
+        (USER_CONF / '0/jupyter_config.py')
     ]
 
 
@@ -70,8 +79,8 @@ def test_cylc_site_conf_path(clear_env, capload, monkeypatch):
         SYS_CONF,
         Path('elephant/uiserver/jupyter_config.py'),
         Path('elephant/uiserver/0/jupyter_config.py'),
-        Path(USER_CONF/'jupyter_config.py'),
-        Path(USER_CONF/'0/jupyter_config.py')
+        Path(USER_CONF / 'jupyter_config.py'),
+        Path(USER_CONF / '0/jupyter_config.py')
     ]
 
 

cylc/uiserver/tests/test_data_store_mgr.py

@@ -29,7 +29,6 @@
 from .conftest import AsyncClientFixture
 
 
-@pytest.mark.asyncio
 async def test_entire_workflow_update(
     async_client: AsyncClientFixture,
     data_store_mgr: DataStoreMgr,
@@ -61,7 +60,6 @@ async def test_entire_workflow_update(
     assert entire_workflow.workflow.id == w_id_data['workflow'].id
 
 
-@pytest.mark.asyncio
 async def test_entire_workflow_update_ignores_timeout_message(
     async_client: AsyncClientFixture,
     data_store_mgr: DataStoreMgr
@@ -89,7 +87,6 @@ async def test_entire_workflow_update_ignores_timeout_message(
     assert w_id not in data_store_mgr.data
 
 
-@pytest.mark.asyncio
 async def test_entire_workflow_update_gather_error(
     async_client: AsyncClientFixture,
     data_store_mgr: DataStoreMgr,
@@ -128,7 +125,6 @@ async def test_entire_workflow_update_gather_error(
     assert caplog.records[0].exc_info[0] == error_type
 
 
-@pytest.mark.asyncio
 async def test_register_workflow(
     data_store_mgr: DataStoreMgr
 ):
@@ -141,7 +137,6 @@ async def test_register_workflow(
     assert w_id in data_store_mgr.delta_queues
 
 
-@pytest.mark.asyncio
 async def test_update_contact_no_contact_data(
     data_store_mgr: DataStoreMgr
 ):
@@ -154,7 +149,6 @@ async def test_update_contact_no_contact_data(
     assert api_version == data_store_mgr.data[w_id]['workflow'].api_version
 
 
-@pytest.mark.asyncio
 async def test_update_contact_no_workflow(
     data_store_mgr: DataStoreMgr
 ):
@@ -165,7 +159,6 @@ async def test_update_contact_no_workflow(
     assert not data_store_mgr._update_contact(w_id='elephant')
 
 
-@pytest.mark.asyncio
 async def test_update_contact_with_contact_data(
     data_store_mgr: DataStoreMgr
 ):
@@ -185,7 +178,6 @@ async def test_update_contact_with_contact_data(
     assert api_version == data_store_mgr.data[w_id]['workflow'].api_version
 
 
-@pytest.mark.asyncio
 async def test_disconnect_workflow(
     data_store_mgr: DataStoreMgr
 ):
@@ -208,7 +200,6 @@ async def test_disconnect_workflow(
     assert data_store_mgr.data[w_id]['workflow'].api_version == 0
 
 
-@pytest.mark.asyncio
 async def test_workflow_connect_fail(
     data_store_mgr: DataStoreMgr,
     port_range,
@@ -227,10 +218,8 @@ async def test_workflow_connect_fail(
     """
     # patch the zmq logic so that the connection doesn't fail at the first
     # hurdle
-    def _null(*args, **kwargs): pass
     monkeypatch.setattr(
-        'cylc.flow.network.ZMQSocketBase._socket_bind',
-        _null,
+        'cylc.flow.network.ZMQSocketBase._socket_bind', lambda *a, **k: None,
     )
 
     # start a ZMQ REPLY socket in order to claim an unused port