Merge pull request #1 from cdcadman/pr_workflow

Add PR workflow

.github/workflows/code_checks.yml

@@ -0,0 +1,20 @@
+on:
+    pull_request:
+        branches: main
+    schedule:
+      - cron: '25 2 * * 6'
+jobs:
+    code-checks:
+        runs-on: ubuntu-latest
+        steps:
+        - uses: actions/checkout@v4
+        - name: Install python
+          uses: actions/setup-python@v5
+          with:
+            python-version: '3.12'
+        - name: Install python packages
+          run: |
+            python3.12 -m pip install --upgrade pip
+            python3.12 -m pip install -r requirements-dev.txt
+        - name: Run code checks
+          run: ./code_checks.sh

CONTRIBUTING.md

@@ -1,3 +1,48 @@
 # Contributing to 247 bishops
 
 By contributing to 247 bishops, you agree that your contributions will be licensed under its GNU AGPL 3.
+If you are adding something to this web app which is not your own, it must either in the public domain or have
+a license which is compatible with the GNU AGPL 3.
+
+## Guidelines
+
+To contribute code changes to this repo, create a pull request into the main branch.
+The following checks have to pass before it can merge (defined in `code_checks.yml`).
+
+* licensecheck - confirms that all python packages in requirements.txt are compatible with this project's license.
+* pip_audit - confirms that no python packages used by this project (including in `requirements-dev.txt`) have reported vulnerabilities.
+  * If any vulnerabilities are reported and if there is a fix version, update `requirements.txt` by running `calc_deterministic.sh`.
+* bandit - checks for unsafe python code in this repo.
+  * To ignore a reported issue, please use the code.  For example, putting `# nosec B608` on a line will ignore a potential SQL injection issue.
+  * If you have to insert the value of a variable into a query string, please ensure that the variable is trusted or checked.  For example:
+
+```python
+# `positions` is an untrusted list of strings.  The values are passed as parameters, but the right number of markers needs to be inserted into the query.
+result = conn.exec_driver_sql(
+    f"select * from position_data where position in ({', '.join('%s' for _ in positions)})",  # nosec B608
+    tuple(positions),
+)
+```
+
+* black - enforces black formatting.  Please run `python -m black .` before creating a pull request.
+* isort - enforces import sorting.  Please run `python -m isort .` before creating a pull request.
+* pylint - checks for errors or warnings.  If you have to ignore a message, please include the code.
+For example: `from webapp_python import app  # pylint:disable=unused-import`
+* pytest - fails on warnings and checks for 100% code coverage.
+  * If you are unable to prevent a warning, please ignore it in `pyproject.toml` in `filterwarnings` using the precise line number.
+  For example: `"ignore:Use list:DeprecationWarning:msal.token_cache:164",`
+  * If you are unable to ensure 100% code coverage, please use `# pragma: no cover` sparingly, preferably only in tests.  For example:
+
+```python
+# expected_condition() should return True, possibly after a brief delay
+while True:
+    if expected_condition():
+        break
+    time.sleep(0.1) # pragma: no cover
+```
+
+## Adding packages
+
+To add a python package, place it in `base_reqs.txt` with no version specifier.  Then run `calc_deterministic.sh` to update `requirements.txt`.
+
+If you are adding a javascript package in an html file, please ensure that it has an open source license compatible with the GNU AGPL 3.

app.py

@@ -1 +1 @@
-from webapp_python import app
+from webapp_python import app  # pylint:disable=unused-import

code_checks.sh

@@ -0,0 +1,10 @@
+#!/bin/bash
+set -e
+python3.12 -m licensecheck --zero
+python3.12 -m pip_audit
+python3.12 -m bandit -c pyproject.toml -r .
+python3.12 -m black . --check
+python3.12 -m isort . --check
+python3.12 -m pylint .
+python3.12 -m pytest --cov=. --cov-report=term-missing
+python3.12 -m coverage report --fail-under=100

pyproject.toml

@@ -0,0 +1,21 @@
+[tool.bandit]
+exclude_dirs = ["tests", "run_flask.py", "venv"]
+
+[tool.pytest.ini_options]
+filterwarnings=[
+    "error",
+]
+
+[tool.licensecheck]
+using = "requirements:requirements.txt"
+
+[project]
+license = "AGPL"
+
+[tool.isort]
+profile = "black"
+
+[tool.pylint]
+ignore = ["venv"]
+recursive = "y"
+disable = "C,R,I"

requirements-dev.txt

@@ -0,0 +1,10 @@
+-r requirements.txt
+pytest>=7.3.2
+pytest-cov>=4.1.0
+pylint>=2.17.4
+black>=23.3.0
+isort>=5.12.0
+pip-audit>=2.7.0
+bandit>=1.7.5
+licensecheck>=2024
+requests>=2.32.2

run_flask.py

@@ -3,7 +3,9 @@
 `python run_flask.py 5001` runs it on port 5001
 Direct your webbrowser to http://127.0.0.1:5001 to view the app (if you ran it on port 5001).
 """
+
 import sys
+
 from app import app
 
 PORT = sys.argv[1] if len(sys.argv) > 1 else 5000

tests/server.py

@@ -0,0 +1,42 @@
+import signal
+import subprocess as sp
+import sys
+from contextlib import contextmanager
+
+TIMEOUT = 5
+
+
+def close_server(server: sp.Popen, check_status=True):
+    server.send_signal(signal.SIGINT)
+    for line in iter(server.stderr.readline, ""):
+        sys.stderr.write(line)
+    server.stderr.close()
+    assert server.wait(TIMEOUT) in (0, -2) or not check_status
+
+
+@contextmanager
+def get_server_url():
+    port = 5000
+    while True:
+        server = sp.Popen(
+            [
+                sys.executable,
+                "run_flask.py",
+                str(port),
+            ],
+            stderr=sp.PIPE,
+            text=True,
+            universal_newlines=True,
+            bufsize=1,
+        )
+        out = server.stderr.readline()
+        sys.stderr.write(out)
+        if "Address already in use" in out:
+            close_server(server, check_status=False)
+            port += 1
+        else:
+            break
+    try:
+        yield f"http://127.0.0.1:{port}"
+    finally:
+        close_server(server)

tests/test_main.py

@@ -0,0 +1,12 @@
+import requests
+
+from .server import get_server_url
+
+
+def test_main():
+    with get_server_url() as webapp_url:
+        response = requests.get(f"{webapp_url}", timeout=60)
+        assert response.status_code == 200
+        assert "24/7 Bishops" in response.text
+        response = requests.get(f"{webapp_url}/favicon.svg?v=1729", timeout=60)
+        assert response.status_code == 200

tests/test_server.py

@@ -0,0 +1,9 @@
+from .server import get_server_url
+
+
+def test_port_increment():
+    with get_server_url() as first_url:
+        first_port = int(first_url.split(":")[2])
+        with get_server_url() as second_url:
+            second_port = int(second_url.split(":")[2])
+            assert second_port > first_port

webapp_python/main.py

@@ -1,14 +1,21 @@
 from pathlib import Path
+
 import flask
 
 app = flask.Flask(__name__)
 
+
 @app.route("/")
 def main():
-    return flask.send_file(Path(__file__).parent.absolute().parent / "html" / "main.html")
+    return flask.send_file(
+        Path(__file__).parent.absolute().parent / "html" / "main.html"
+    )
+
 
 @app.route("/favicon.svg")
 def favicon():
     """If this is updated, you also need to increment the version in the link tag in the html file.
     See https://stackoverflow.com/questions/2208933/how-do-i-force-a-favicon-refresh"""
-    return flask.send_file(Path(__file__).parent.absolute().parent / "html" / "Chess_tile_bl.svg")
+    return flask.send_file(
+        Path(__file__).parent.absolute().parent / "html" / "Chess_tile_bl.svg"
+    )