add flake8-isort, with a caveat

if the client has requested /foo, and foo is actually a directory,
this redirects the client to /foo/

.gitignore

@@ -12,6 +12,7 @@ __pycache__/
 build/
 develop-eggs/
 dist/
+dist
 downloads/
 eggs/
 .eggs/

MANIFEST.in

@@ -1,5 +1,4 @@
 graft incorporealcms/static
 graft incorporealcms/templates
-include versioneer.py
 global-exclude *.pyc
 global-exclude *.swp

incorporealcms/__init__.py

@@ -1,4 +1,4 @@
-"""create_app application factory function and similar things."""
+"""An application for running my Markdown-based sites."""
 import logging
 import os
 from logging.config import dictConfig
@@ -12,6 +12,7 @@ del get_versions
 
 
 def create_app(instance_path=None, test_config=None):
+    """Create the Flask app, with allowances for customizing path and test settings."""
     app = Flask(__name__, instance_relative_config=True, instance_path=instance_path)
 
     # if it doesn't already exist, create the instance folder

incorporealcms/pages.py

@@ -4,7 +4,9 @@ import logging
 import os
 
 import markdown
-from flask import Blueprint, Markup, abort, current_app as app, render_template
+from flask import Blueprint, Markup, abort
+from flask import current_app as app
+from flask import redirect, render_template
 from tzlocal import get_localzone
 
 logger = logging.getLogger(__name__)
@@ -17,6 +19,9 @@ md = markdown.Markdown(extensions=['meta', 'tables'])
 @bp.route('/<path:path>')
 def display_page(path):
     """Get the file contents of the requested path and render the file."""
+    if is_file_path_actually_dir_path(path):
+        return redirect(f'{path}/', code=301)
+
     resolved_path = resolve_page_file(path)
     logger.debug("received request for path '%s', resolved to '%s'", path, resolved_path)
     try:
@@ -50,6 +55,22 @@ def resolve_page_file(path):
     return path
 
 
+def is_file_path_actually_dir_path(path):
+    """Check if requested path which looks like a file is actually a directory.
+
+    If, for example, /foo used to be a file (foo.md) which later became a directory,
+    foo/, this returns True. Useful for when I make my structure more complicated
+    than it originally was, or if users are just weird.
+    """
+    if not path.endswith('/'):
+        logger.debug("requested path '%s' looks like a file", path)
+        if os.path.isdir(f'{app.instance_path}/pages/{path}'):
+            logger.debug("...and it's actually a dir")
+            return True
+
+    return False
+
+
 def generate_parent_navs(path):
     """Create a series of paths/links to navigate up from the given path."""
     # derive additional path/location stuff based on path

requirements/requirements-dev.in

@@ -1,7 +1,13 @@
 -r requirements.in
 
-flake8                      # python code quality stuff
+flake8                      # flake8 and plugins, for local dev linting in vim
+flake8-blind-except
+flake8-builtins
+flake8-docstrings
+flake8-executable
+flake8-isort
+flake8-logging-format
 pip-tools                   # pip-compile
-pytest                      # unit tests
-pytest-cov                  # coverage in unit tests
+tox                         # CI stuff
+tox-wheel                   # build wheels in tox
 versioneer                  # automatic version numbering

requirements/requirements-dev.txt

@@ -4,34 +4,47 @@
 #
 #    pip-compile --output-file=requirements/requirements-dev.txt requirements/requirements-dev.in
 #
-attrs==19.3.0             # via pytest
+appdirs==1.4.4            # via virtualenv
 click==7.1.2              # via flask, pip-tools
-coverage==5.1             # via pytest-cov
-flake8==3.8.2             # via -r requirements/requirements-dev.in
+distlib==0.3.0            # via virtualenv
+filelock==3.0.12          # via tox, virtualenv
+flake8-blind-except==0.1.1  # via -r requirements/requirements-dev.in
+flake8-builtins==1.5.3    # via -r requirements/requirements-dev.in
+flake8-docstrings==1.5.0  # via -r requirements/requirements-dev.in
+flake8-executable==2.0.3  # via -r requirements/requirements-dev.in
+flake8-isort==3.0.0       # via -r requirements/requirements-dev.in
+flake8-logging-format==0.6.0  # via -r requirements/requirements-dev.in
+flake8==3.8.3             # via -r requirements/requirements-dev.in, flake8-builtins, flake8-docstrings, flake8-executable, flake8-isort
 flask==1.1.2              # via -r requirements/requirements.in
-importlib-metadata==1.6.0  # via flake8, markdown, pluggy, pytest
+importlib-metadata==1.6.0  # via flake8, markdown, pluggy, tox, virtualenv
+isort[pyproject]==4.3.21  # via flake8-isort
 itsdangerous==1.1.0       # via flask
 jinja2==2.11.2            # via flask
 markdown==3.2.2           # via -r requirements/requirements.in
 markupsafe==1.1.1         # via jinja2
 mccabe==0.6.1             # via flake8
-more-itertools==8.3.0     # via pytest
-packaging==20.4           # via pytest
-pip-tools==5.2.0          # via -r requirements/requirements-dev.in
-pluggy==0.13.1            # via pytest
-py==1.8.1                 # via pytest
+packaging==20.4           # via tox
+pip-tools==5.2.1          # via -r requirements/requirements-dev.in
+pluggy==0.13.1            # via tox
+py==1.8.1                 # via tox
 pycodestyle==2.6.0        # via flake8
+pydocstyle==5.0.2         # via flake8-docstrings
 pyflakes==2.2.0           # via flake8
 pyparsing==2.4.7          # via packaging
-pytest-cov==2.9.0         # via -r requirements/requirements-dev.in
-pytest==5.4.2             # via -r requirements/requirements-dev.in, pytest-cov
 pytz==2020.1              # via tzlocal
-six==1.15.0               # via packaging, pip-tools
+six==1.15.0               # via packaging, pip-tools, tox, virtualenv
+snowballstemmer==2.0.0    # via pydocstyle
+testfixtures==6.14.1      # via flake8-isort
+toml==0.10.1              # via isort, tox
+tox-wheel==0.4.2          # via -r requirements/requirements-dev.in
+tox==3.15.2               # via -r requirements/requirements-dev.in, tox-wheel
 tzlocal==2.1              # via -r requirements/requirements.in
 versioneer==0.18          # via -r requirements/requirements-dev.in
-wcwidth==0.1.9            # via pytest
+virtualenv==20.0.23       # via tox
 werkzeug==1.0.1           # via flask
+wheel==0.34.2             # via tox-wheel
 zipp==3.1.0               # via importlib-metadata
 
 # The following packages are considered to be unsafe in a requirements file:
 # pip
+# setuptools

setup.cfg

@@ -1,23 +1,3 @@
-[coverage:run]
-branch = True
-omit =
-    .venv/*
-    incorporealcms/_version.py
-    setup.py
-    tests/*
-    versioneer.py
-source =
-    incorporealcms
-
-[flake8]
-exclude = .git,.venv,__pycache__,versioneer.py,incorporealcms/_version.py
-max-line-length = 120
-
-[tool:pytest]
-addopts = --cov=. --cov-report=term --cov-report=term-missing
-log_cli = 1
-log_cli_level = DEBUG
-
 [versioneer]
 VCS = git
 style = pep440-post

setup.py

@@ -1,5 +1,6 @@
 """Setuptools configuration."""
 import os
+
 from setuptools import find_packages, setup
 
 import versioneer

tests/conftest.py

@@ -10,6 +10,7 @@ HERE = os.path.dirname(os.path.abspath(__file__))
 
 @pytest.fixture
 def app():
+    """Create the Flask application, with test settings."""
     app = create_app(instance_path=os.path.join(HERE, 'instance'))
 
     yield app
@@ -17,4 +18,5 @@ def app():
 
 @pytest.fixture
 def client(app):
+    """Create a test client based on the test app."""
     return app.test_client()

tests/functional_tests.py

@@ -0,0 +1,54 @@
+"""Test page requests."""
+import re
+
+
+def test_page_that_exists(client):
+    """Test that the app can serve a basic file at the index."""
+    response = client.get('/')
+    assert response.status_code == 200
+    assert b'<h1>test index</h1>' in response.data
+
+
+def test_page_that_doesnt_exist(client):
+    """Test that the app returns 404 for nonsense requests."""
+    response = client.get('/ohuesthaoeusth')
+    assert response.status_code == 404
+
+
+def test_page_with_title_metadata(client):
+    """Test that a page with title metadata has its title written."""
+    response = client.get('/')
+    assert response.status_code == 200
+    assert b'<title>Index - incorporeal.org</title>' in response.data
+
+
+def test_page_without_title_metadata(client):
+    """Test that a page without title metadata gets the default title."""
+    response = client.get('/no-title')
+    assert response.status_code == 200
+    assert b'<title>incorporeal.org</title>' in response.data
+    assert b'<h1>this page doesn\'t have a title!</h1>' in response.data
+
+
+def test_page_has_modified_timestamp(client):
+    """Test that pages have modified timestamps in them."""
+    response = client.get('/')
+    assert response.status_code == 200
+    assert re.search(r'Last modified: ....-..-.. ..:..:.. ...', response.data.decode()) is not None
+
+
+def test_that_page_request_redirects_to_directory(client):
+    """Test that a request to /foo reirects to /foo/, if foo is a directory.
+
+    This might be useful in cases where a formerly page-only page has been
+    converted to a directory with subpages.
+    """
+    response = client.get('/subdir')
+    assert response.status_code == 301
+
+
+def test_that_dir_request_does_not_redirect(client):
+    """Test that a request to /foo/ serves the index page, if foo is a directory."""
+    response = client.get('/subdir/')
+    assert response.status_code == 200
+    assert b'another page' in response.data

tests/instance/config.py

@@ -1,3 +1,5 @@
+"""Configure the test application."""
+
 LOGGING = {
     'version': 1,
     'formatters': {

tests/test_factory.py

@@ -24,6 +24,7 @@ def test_title_override():
 
 
 def test_media_file_access(client):
+    """Test that media files are served, and properly."""
     response = client.get('/media/favicon.png')
     assert response.status_code == 200
     assert response.headers['content-type'] == 'image/png'

tests/test_pages.py

@@ -1,76 +1,76 @@
-"""Test page views and helper methods."""
-import re
-
-from incorporealcms.pages import generate_parent_navs, resolve_page_file
+"""Unit test helper methods."""
+from incorporealcms.pages import generate_parent_navs, is_file_path_actually_dir_path, resolve_page_file
 
 
 def test_resolve_page_file_dir_to_index():
+    """Test that a request to a directory path results in the dir's index.md."""
     assert resolve_page_file('foo/') == 'pages/foo/index.md'
 
 
 def test_resolve_page_file_subdir_to_index():
+    """Test that a request to a dir's subdir path results in the subdir's index.md."""
     assert resolve_page_file('foo/bar/') == 'pages/foo/bar/index.md'
 
 
 def test_resolve_page_file_other_requests_fine():
+    """Test that a request to non-dir path results in a Markdown file."""
     assert resolve_page_file('foo/baz') == 'pages/foo/baz.md'
 
 
-def test_page_that_exists(client):
-    response = client.get('/')
-    assert response.status_code == 200
-    assert b'<h1>test index</h1>' in response.data
-
-
-def test_page_that_doesnt_exist(client):
-    response = client.get('/ohuesthaoeusth')
-    assert response.status_code == 404
-
-
-def test_page_with_title_metadata(client):
-    response = client.get('/')
-    assert response.status_code == 200
-    assert b'<title>Index - incorporeal.org</title>' in response.data
-
-
-def test_page_without_title_metadata(client):
-    response = client.get('/no-title')
-    assert response.status_code == 200
-    assert b'<title>incorporeal.org</title>' in response.data
-    assert b'<h1>this page doesn\'t have a title!</h1>' in response.data
-
-
-def test_page_has_modified_timestamp(client):
-    response = client.get('/')
-    assert response.status_code == 200
-    assert re.search(r'Last modified: ....-..-.. ..:..:.. ...', response.data.decode()) is not None
-
-
 def test_generate_page_navs_index(app):
+    """Test that the index page has navs to the root (itself)."""
     with app.app_context():
         assert generate_parent_navs('/') == [('incorporeal.org', '/')]
 
 
 def test_generate_page_navs_alternate_index(app):
+    """Test that the index page (as a page, not a path) also has navs only to the root (by path)."""
     with app.app_context():
         assert generate_parent_navs('index') == [('incorporeal.org', '/')]
 
 
 def test_generate_page_navs_subdir_index(app):
+    """Test that dir pages have navs to the root and themselves."""
     with app.app_context():
         assert generate_parent_navs('subdir/') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/')]
 
 
 def test_generate_page_navs_subdir_real_page(app):
+    """Test that real pages have navs to the root, their parent, and themselves."""
     with app.app_context():
         assert generate_parent_navs('subdir/page') == [('incorporeal.org', '/'), ('/subdir/', '/subdir/'),
                                                        ('Page', '/subdir/page')]
 
 
 def test_generate_page_navs_subdir_with_title_parsing_real_page(app):
+    """Test that title metadata is used in the nav text."""
     with app.app_context():
         assert generate_parent_navs('subdir-with-title/page') == [
             ('incorporeal.org', '/'),
             ('SUB!', '/subdir-with-title/'),
             ('/subdir-with-title/page', '/subdir-with-title/page')
         ]
+
+
+def test_is_file_path_actually_dir_path_valid_file_is_yes(app):
+    """Test that a file request for what's actually a directory is detected as such."""
+    with app.app_context():
+        assert is_file_path_actually_dir_path('/subdir')
+
+
+def test_is_file_path_actually_dir_path_valid_dir_is_no(app):
+    """Test that a directory request is still a directory request."""
+    with app.app_context():
+        assert not is_file_path_actually_dir_path('/subdir/')
+
+
+def test_is_file_path_actually_dir_path_nonsense_file_is_no(app):
+    """Test that requests to nonsense file-looking paths aren't treated as dirs."""
+    with app.app_context():
+        assert not is_file_path_actually_dir_path('/antphnathpnthapnthsnthax')
+
+
+def test_is_file_path_actually_dir_path_nonsense_dir_is_no(app):
+    """Test that a directory request is a directory request even if the dir doesn't exist."""
+    with app.app_context():
+        assert not is_file_path_actually_dir_path('/antphnathpnthapnthsnthax/')

tox.ini

@@ -0,0 +1,84 @@
+# tox (https://tox.readthedocs.io/) is a tool for running tests
+# in multiple virtualenvs. This configuration file will run the
+# test suite on all supported python versions. To use it, "pip install tox"
+# and then run "tox" from this directory.
+
+[tox]
+envlist = py37,security,lint
+
+[testenv]
+# build a wheel and test it
+wheel = true
+wheel_build_env = build
+
+# whitelist commands we need
+whitelist_externals = ln
+
+[testenv:build]
+# require setuptools when building
+deps = setuptools
+
+[testenv:py37]
+# run pytest and coverage
+deps =
+    pytest
+    pytest-cov
+commands =
+    pytest --cov={envsitepackagesdir}/incorporealcms/ \
+           --cov-report=term-missing --cov-branch --cov-fail-under=90
+    ln -sf {distdir} dist
+
+[testenv:security]
+# run security checks
+#
+# again it seems the most valuable here to run against the packaged code
+deps =
+    bandit
+commands =
+    bandit {envsitepackagesdir}/incorporealcms/ -r
+
+[testenv:lint]
+# run style checks
+skip_install = true
+deps =
+    flake8
+    flake8-blind-except
+    flake8-builtins
+    flake8-docstrings
+    flake8-executable
+    flake8-isort
+    flake8-logging-format
+commands =
+    flake8
+    - flake8 --disable-noqa --select=E,W,F,C,D,A,G,B,I
+
+[coverage:paths]
+source =
+    ./
+    .tox/**/site-packages/
+
+[coverage:run]
+branch = True
+omit =
+    **/_version.py
+
+[flake8]
+max-line-length = 120
+exclude =
+    .tox/
+    versioneer.py
+    _version.py
+    instance/
+max-complexity = 10
+
+[isort]
+line_length = 120
+# only way I could figure out how to make tests/conftest.py happy and have pytest separate from incorporealcms
+# and if I let them be combined in tox runs, flake8-isort in vim would complain :(
+forced_separate = incorporealcms
+
+[pytest]
+python_files =
+    *_tests.py
+    tests.py
+    test_*.py