Add flake8 and handle warnings in tests (#259)

blink1073 · web-flow · commit a88173e56499 · 2022-04-01T03:52:21.000-05:00
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
@@ -49,16 +49,16 @@ repos:
       - id: doc8
         args: [--max-line-length=200]
 
-  # - repo: https://github.com/pycqa/flake8
-  #   rev: 4.0.1
-  #   hooks:
-  #     - id: flake8
-  #       additional_dependencies:
-  #         [
-  #           "flake8-bugbear==20.1.4",
-  #           "flake8-logging-format==0.6.0",
-  #           "flake8-implicit-str-concat==0.2.0",
-  #         ]
+  - repo: https://github.com/pycqa/flake8
+    rev: 4.0.1
+    hooks:
+      - id: flake8
+        additional_dependencies:
+          [
+            "flake8-bugbear==20.1.4",
+            "flake8-logging-format==0.6.0",
+            "flake8-implicit-str-concat==0.2.0",
+          ]
 
   - repo: https://github.com/sirosen/check-jsonschema
     rev: 0.14.1
diff --git a/jupyterlab_server/__init__.py b/jupyterlab_server/__init__.py
@@ -5,7 +5,7 @@
 from .app import LabServerApp
 from .handlers import LabConfig, LabHandler, add_handlers
 from .licenses_app import LicensesApp
-from .spec import get_openapi_spec, get_openapi_spec_dict
+from .spec import get_openapi_spec, get_openapi_spec_dict  # noqa
 from .translation_utils import translator
 from .workspaces_app import WorkspaceExportApp, WorkspaceImportApp, WorkspaceListApp
 from .workspaces_handler import WORKSPACE_EXTENSION, slugify
diff --git a/jupyterlab_server/app.py b/jupyterlab_server/app.py
@@ -27,7 +27,7 @@ def app_namespace(self):
     # Should your extension expose other server extensions when launched directly?
     load_other_extensions = True
 
-    app_version = Unicode("", help="The version of the application.", default=__version__)
+    app_version = Unicode("", help="The version of the application.").tag(default=__version__)
 
     blacklist_uris = Unicode(
         "", config=True, help="Deprecated, use `LabServerApp.blocked_extensions_uris`"
diff --git a/jupyterlab_server/config.py b/jupyterlab_server/config.py
@@ -113,9 +113,9 @@ def get_page_config(labextensions_path, app_settings_dir=None, logger=None):
     # Ensure there is a disabled key
     page_config.setdefault(disabled_key, {})
 
-    for (ext, ext_data) in federated_exts.items():
-        if not "_build" in ext_data["jupyterlab"]:
-            logger.warn("%s is not a valid extension" % ext_data["name"])
+    for (_, ext_data) in federated_exts.items():
+        if "_build" not in ext_data["jupyterlab"]:
+            logger.warning("%s is not a valid extension" % ext_data["name"])
             continue
         extbuild = ext_data["jupyterlab"]["_build"]
         extension = {"name": ext_data["name"], "load": extbuild["load"]}
@@ -152,7 +152,7 @@ def get_page_config(labextensions_path, app_settings_dir=None, logger=None):
     disabled_by_extensions = {}
     for name in sorted(disabled_by_extensions_all):
         # skip if the extension itself is disabled by other config
-        if page_config[disabled_key].get(name) == True:
+        if page_config[disabled_key].get(name) is True:
             continue
 
         disabled_list = disabled_by_extensions_all[name]
@@ -217,7 +217,7 @@ class LabConfig(HasTraits):
     settings_url = Unicode(help="The url path of the settings handler.").tag(config=True)
 
     user_settings_dir = Unicode(
-        "", help=("The optional location of the user " "settings directory.")
+        "", help=("The optional location of the user settings directory.")
     ).tag(config=True)
 
     schemas_dir = Unicode(
@@ -263,7 +263,7 @@ class LabConfig(HasTraits):
 
     cache_files = Bool(
         True,
-        help=("Whether to cache files on the server. " "This should be `True` except in dev mode."),
+        help=("Whether to cache files on the server. This should be `True` except in dev mode."),
     ).tag(config=True)
 
     notebook_starts_kernel = Bool(
@@ -314,10 +314,6 @@ def _default_tree_url(self):
     def _default_translations_api_url(self):
         return ujoin(self.app_url, "api", "translations/")
 
-    @default("tree_url")
-    def _default_tree_url(self):
-        return ujoin(self.app_url, "tree/")
-
 
 def _get_config_manager(level):
     """Get the location of config files for the current context
diff --git a/jupyterlab_server/handlers.py b/jupyterlab_server/handlers.py
@@ -224,7 +224,7 @@ def add_handlers(handlers, extension_app):
         handlers.append((setting_path, SettingsHandler, settings_config))
 
         # Handle translations.
-        ## Translations requires settings as the locale source of truth is stored in it
+        # Translations requires settings as the locale source of truth is stored in it
         if extension_app.translations_api_url:
             # Handle requests for the list of language packs available.
             # Make slash optional.
diff --git a/jupyterlab_server/licenses_app.py b/jupyterlab_server/licenses_app.py
@@ -76,7 +76,6 @@ def initialize(self, *args, **kwargs):
 
     def init_licenses_manager(self):
         self.licenses_manager = LicensesManager(
-            labextensions_path=sum([self.labextensions_path + self.extra_labextensions_path], []),
             parent=self,
         )
 
diff --git a/jupyterlab_server/process.py b/jupyterlab_server/process.py
@@ -95,7 +95,7 @@ def __init__(self, cmd, logger=None, cwd=None, kill_event=None, env=None, quiet=
         self.logger = logger = logger or logging.getLogger("jupyterlab")
         self._last_line = ""
         if not quiet:
-            self.logger.info("> " + list2cmdline(cmd))
+            self.logger.info(f"> {list2cmdline(cmd)}")
         self.cmd = cmd
 
         kwargs = {}
diff --git a/jupyterlab_server/pytest_plugin.py b/jupyterlab_server/pytest_plugin.py
@@ -7,7 +7,6 @@
 import pytest
 
 from jupyterlab_server import LabServerApp
-from jupyterlab_server.app import LabServerApp
 
 pytest_plugins = ["jupyter_server.pytest_plugin"]
 
diff --git a/jupyterlab_server/server.py b/jupyterlab_server/server.py
@@ -1,4 +1,4 @@
-from jupyter_server import _tz as tz
+from jupyter_server import _tz as tz  # noqa
 from jupyter_server.base.handlers import (  # noqa
     APIHandler,
     FileFindHandler,
diff --git a/jupyterlab_server/settings_handler.py b/jupyterlab_server/settings_handler.py
@@ -12,7 +12,7 @@
 from tornado import web
 
 from .settings_utils import SchemaHandler, get_settings, save_settings
-from .translation_utils import DEFAULT_LOCALE, translator
+from .translation_utils import translator
 
 
 class SettingsHandler(ExtensionHandlerMixin, ExtensionHandlerJinjaMixin, SchemaHandler):
@@ -45,7 +45,7 @@ def get(self, schema_name=""):
         # Print all warnings.
         for w in warnings:
             if w:
-                self.log.warn(w)
+                self.log.warning(w)
 
         return self.finish(json.dumps(result))
 
@@ -78,7 +78,7 @@ def put(self, schema_name):
             )
         except json.decoder.JSONDecodeError as e:
             raise web.HTTPError(400, invalid_json_error % str(e))
-        except (KeyError, TypeError) as e:
+        except (KeyError, TypeError):
             raise web.HTTPError(400, invalid_payload_format_error)
         except ValidationError as e:
             raise web.HTTPError(400, validation_error % str(e))
diff --git a/jupyterlab_server/settings_utils.py b/jupyterlab_server/settings_utils.py
@@ -429,7 +429,7 @@ def initialize(self, app_settings_dir, schemas_dir, settings_dir, labextensions_
 
         if error:
             overrides_warning = "Failed loading overrides: %s"
-            self.log.warn(overrides_warning % str(error))
+            self.log.warning(overrides_warning % str(error))
 
     def get_current_locale(self):
         """
@@ -455,7 +455,7 @@ def get_current_locale(self):
             )
         except web.HTTPError as e:
             schema_warning = "Missing or misshappen translation settings schema:\n%s"
-            self.log.warn(schema_warning % str(e))
+            self.log.warning(schema_warning % str(e))
 
             settings = {}
 
diff --git a/jupyterlab_server/test_utils.py b/jupyterlab_server/test_utils.py
@@ -159,4 +159,4 @@ def assert_http_error(status, msg=None):
         if msg:
             assert msg in str(e), e
     else:
-        assert False, "Expected HTTP error status"
+        raise AssertionError("Expected HTTP error status")
diff --git a/jupyterlab_server/themes_handler.py b/jupyterlab_server/themes_handler.py
@@ -75,7 +75,7 @@ def _get_css(self):
         # Replace local paths with mangled paths.
         # We only match strings that are local urls,
         # e.g. `url('../foo.css')`, `url('images/foo.png')`
-        pattern = r"url\('(.*)'\)|" r'url\("(.*)"\)'
+        pattern = r"url\('(.*)'\)|url\('(.*)'\)"
 
         def replacer(m):
             """Replace the matched relative url with the mangled url."""
diff --git a/jupyterlab_server/translation_utils.py b/jupyterlab_server/translation_utils.py
@@ -654,7 +654,7 @@ def set_locale(cls, locale: str):
         if is_valid_locale(locale):
             cls._LOCALE = locale
             translator._update_env(locale)
-            for domain, bundle in cls._TRANSLATORS.items():
+            for _, bundle in cls._TRANSLATORS.items():
                 bundle.update_locale(locale)
 
     @classmethod
@@ -699,7 +699,7 @@ def _translate_schema_strings(
 
             if isinstance(value, str):
                 matched = False
-                for pattern, context in to_translate.items():
+                for pattern, context in to_translate.items():  # noqa
                     if pattern.fullmatch(path):
                         matched = True
                         break
diff --git a/jupyterlab_server/workspaces_app.py b/jupyterlab_server/workspaces_app.py
@@ -92,7 +92,7 @@ def start(self):
         try:
             workspace = self.manager.load(raw)
             print(json.dumps(workspace))
-        except Exception as e:
+        except Exception:
             print(json.dumps(dict(data=dict(), metadata=dict(id=raw))))
 
 
diff --git a/pyproject.toml b/pyproject.toml
@@ -24,5 +24,15 @@ tag_template = "v{new_version}"
 src = "jupyterlab_server/_version.py"
 
 [tool.pytest.ini_options]
-addopts = "--color=yes -s --tb=long -svv --cov jupyterlab_server --cov-report term-missing --cov-report term:skip-covered"
-filterwarnings = ["ignore::DeprecationWarning:notebook", "ignore::DeprecationWarning:traitlets"]
+addopts = "-raXs --durations 10 --color=yes --doctest-modules"
+testpaths = [
+    "tests/"
+]
+timeout = 300
+# Restore this setting to debug failures
+# timeout_method = "thread"
+filterwarnings = [
+  "error",
+  "ignore:There is no current event loop:DeprecationWarning",
+  "ignore:Passing a schema to Validator.iter_errors:DeprecationWarning"
+]
diff --git a/setup.cfg b/setup.cfg
@@ -64,3 +64,21 @@ test =
 exclude =
     docs*
     tests*
+
+[flake8]
+ignore = E501, W503, E402
+builtins = c, get_config
+exclude =
+    .cache,
+    .github,
+    docs,
+    setup.py
+enable-extensions = G
+extend-ignore =
+    G001, G002, G004, G200, G201, G202,
+    # black adds spaces around ':'
+    E203,
+per-file-ignores =
+    # B011: Do not call assert False since python -O removes these calls
+    # F841 local variable 'foo' is assigned to but never used
+    tests/*: B011, F841
diff --git a/tests/test_licenses_api.py b/tests/test_licenses_api.py
@@ -46,7 +46,7 @@ def _make_static_dir(app, tmp_path, has_licenses=True, license_json=None, packag
             encoding="utf-8",
         )
 
-    setattr(app, "static_dir", str(static_dir))
+    app.static_dir = str(static_dir)
 
 
 def _good_license_json():
diff --git a/tests/test_settings_api.py b/tests/test_settings_api.py
@@ -11,7 +11,6 @@
 from jupyterlab_server.test_utils import (
     big_unicode_string,
     expected_http_error,
-    maybe_patch_ioloop,
     validate_request,
 )
 
diff --git a/tests/test_translation_api.py b/tests/test_translation_api.py
@@ -8,11 +8,7 @@
 
 import pytest
 
-from jupyterlab_server.test_utils import (
-    expected_http_error,
-    maybe_patch_ioloop,
-    validate_request,
-)
+from jupyterlab_server.test_utils import maybe_patch_ioloop, validate_request
 from jupyterlab_server.translation_utils import (
     _get_installed_language_pack_locales,
     _get_installed_package_locales,
diff --git a/tests/test_workspaces_api.py b/tests/test_workspaces_api.py
@@ -1,7 +1,6 @@
 """Test the kernels service API."""
 import json
 import os
-import shutil
 
 import pytest
 import tornado

Original file line number	Diff line number	Diff line change
`@@ -76,7 +76,6 @@ def initialize(self, args, *kwargs):`
`76`	`76`
`77`	`77`	`def init_licenses_manager(self):`
`78`	`78`	`self.licenses_manager = LicensesManager(`
`79`		`- labextensions_path=sum([self.labextensions_path + self.extra_labextensions_path], []),`
`80`	`79`	`parent=self,`
`81`	`80`	`)`
`82`	`81`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-from jupyter_server import _tz as tz`
	`1`	`+from jupyter_server import _tz as tz # noqa`
`2`	`2`	`from jupyter_server.base.handlers import ( # noqa`
`3`	`3`	`APIHandler,`
`4`	`4`	`FileFindHandler,`