From 297fac7750a326a5773da4298a83196dbed45d0d Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Wed, 3 May 2023 19:12:52 +0000 Subject: [PATCH] fix: Remove unused files; add more tests; fix IDA docs and lint Remove unused files from root of each cookiecutter. These were not actually used as they were not inside the `{{cookiecutter.repo_name}}` dir that would lead to them being included. More information on removals: - `openedx.yaml` removed from xblock cookiecutter root and https://github.com/openedx/edx-cookiecutters/issues/334 filed to cover actually adding that tag. (The rest of the file was either redundant with what's in python-template, or obsolete.) - `requirements.txt` was not used in CI or baking - Makefiles were not used. This is unfortunate because the IDA one actually had some useful tests in it. I've moved those to the ci.yml workflow for expediency but they should really be put into the unit tests at some point so that they can be run locally. A lot of the django-app Makefile seemed to have been written while the repo was still in development, and relied on packages that are not installed. For the moved tests to pass, I had to make some fixes to the IDA template: - Add missing `docs` target to Makefile - Add some long-line lint suppression where needed Other improvements: - Remove unused `BAKE_OPTIONS` from root Makefile - Fix root Makefile so that it actually uses pip-sync rather than just installing it - Update `JWT_PUBLIC_SIGNING_JWK_SET` to be readable and copyable as JSON (as is likely to be done for edx-platform in the ongoing JWK work) --- .github/workflows/ci.yml | 67 +++++++++++++++++++ Makefile | 12 ++-- cookiecutter-django-app/Makefile | 33 --------- cookiecutter-django-ida/Makefile | 31 --------- cookiecutter-django-ida/requirements.txt | 2 - .../{{cookiecutter.repo_name}}/Makefile | 4 ++ .../settings/devstack.py | 20 ++++-- cookiecutter-xblock/openedx.yaml | 8 --- 8 files changed, 90 insertions(+), 87 deletions(-) delete mode 100644 cookiecutter-django-app/Makefile delete mode 100644 cookiecutter-django-ida/Makefile delete mode 100644 cookiecutter-django-ida/requirements.txt delete mode 100644 cookiecutter-xblock/openedx.yaml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae3422c8..78e0ba33 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,6 +10,9 @@ on: branches: - '**' +defaults: + run: + shell: bash # opts into error checking jobs: run_tests: @@ -38,3 +41,67 @@ jobs: env: TOXENV: ${{ matrix.toxenv }} run: tox + + # Tests that used to be in the cookiecutter-django-ida Makefile but + # that have not yet been moved into the regular unit tests (which + # would require some more work.) + run_ida_tests: + runs-on: ${{ matrix.os }} + strategy: + matrix: + os: [ubuntu-20.04] + python-version: ['3.8'] + + steps: + - uses: actions/checkout@v3 + + - uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + + - name: Generate demo project + run: | + make requirements + cookiecutter cookiecutter-django-ida --no-input + + - name: "Post-gen: Virtualenv, and set up requirements" + working-directory: repo_name + run: | + virtualenv .venv + source .venv/bin/activate + + make upgrade # TODO should be part of initial cookiecutter setup + make requirements + + - name: "Post-gen: Migrations" + working-directory: repo_name + run: | + source .venv/bin/activate + python manage.py makemigrations + make migrate + + - name: "Quality checks" + working-directory: repo_name + run: | + source .venv/bin/activate + make validation_requirements + make validate + + - name: "Ensure translations can be compiled" + working-directory: repo_name + run: | + source .venv/bin/activate + make fake_translations + + - name: "Ensure docker can build a container" + working-directory: repo_name + run: | + source .venv/bin/activate + docker build . --target app + + - name: "Ensure documentation can be compiled" + working-directory: repo_name + run: | + source .venv/bin/activate + make doc_requirements + make docs diff --git a/Makefile b/Makefile index 6d2a3769..8a95cfca 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,5 @@ .PHONY: help quality requirements test upgrade validate -BAKE_OPTIONS=--no-input - help: ## display this help message @echo "Please use \`make ' where is one of" @awk -F ':.*?## ' '/^[a-zA-Z]/ && NF==2 {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort @@ -46,10 +44,12 @@ quality: ## check coding style with pycodestyle and pylint pydocstyle tests isort --check-only --diff */hooks tests -requirements: ## install development environment requirements - pip install -qr requirements/pip.txt - pip install -qr requirements/pip-tools.txt - pip install -r requirements/dev.txt +piptools: ## install pinned version of pip-compile and pip-sync + pip install -r requirements/pip.txt + pip install -r requirements/pip-tools.txt + +requirements: piptools ## install development environment requirements + pip-sync requirements/dev.txt test: ## run tests on every supported Python version tox diff --git a/cookiecutter-django-app/Makefile b/cookiecutter-django-app/Makefile deleted file mode 100644 index 613db3ff..00000000 --- a/cookiecutter-django-app/Makefile +++ /dev/null @@ -1,33 +0,0 @@ -.PHONY: bake help quality replay requirements test upgrade validate watch - -BAKE_OPTIONS=--no-input - -help: ## display this help message - @echo "Please use \`make ' where is one of" - @awk -F ':.*?## ' '/^[a-zA-Z]/ && NF==2 {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort - -bake: ## generate project using defaults - cookiecutter $(BAKE_OPTIONS) . --overwrite-if-exists - -# Define PIP_COMPILE_OPTS=-v to get more information during make upgrade. -PIP_COMPILE = pip-compile --rebuild --upgrade $(PIP_COMPILE_OPTS) - -quality: ## check coding style with pycodestyle and pylint - tox -e quality - -replay: BAKE_OPTIONS=--replay -replay: watch ## replay last cookiecutter run and watch for changes - ; - -requirements: ## install development environment requirements - pip install -qr requirements/pip-tools.txt - pip-sync requirements/dev.txt requirements/private.* - -test: ## run tests on every supported Python version - tox - -validate: ## run tests and quality checks - tox -e quality,py38 - -watch: bake ## generate project using defaults and watch for changes - watchmedo shell-command -p '*.*' -c 'make bake -e BAKE_OPTIONS=$(BAKE_OPTIONS)' -W -R -D \{{cookiecutter.repo_name}}/ diff --git a/cookiecutter-django-ida/Makefile b/cookiecutter-django-ida/Makefile deleted file mode 100644 index e2e760d7..00000000 --- a/cookiecutter-django-ida/Makefile +++ /dev/null @@ -1,31 +0,0 @@ -.DEFAULT_GOAL := test - -.PHONY: requirements test clean - -requirements: - pip install -r requirements.txt - -test: clean - # Create a new project with the default values - cookiecutter . --no-input - - virtualenv -p python3.8 repo_name/.venv - - # Generate requirement pins, install them, and execute the project's tests - . repo_name/.venv/bin/activate && cd repo_name && pip install -U pip==19.3.1 wheel && make upgrade validation_requirements - . repo_name/.venv/bin/activate && cd repo_name && python manage.py makemigrations - . repo_name/.venv/bin/activate && cd repo_name && make migrate validate - - # Ensure translations can be compiled - . repo_name/.venv/bin/activate && cd repo_name && make fake_translations - - # Ensure docker can build a container from this - cd repo_name && docker build . --target app - - # Ensure documentation can be compiled - . repo_name/.venv/bin/activate && cd repo_name && make doc_requirements - . repo_name/.venv/bin/activate && cd repo_name/docs && make html - -clean: - rm -rf .venv - rm -rf repo_name diff --git a/cookiecutter-django-ida/requirements.txt b/cookiecutter-django-ida/requirements.txt deleted file mode 100644 index 5f073f0c..00000000 --- a/cookiecutter-django-ida/requirements.txt +++ /dev/null @@ -1,2 +0,0 @@ -cookiecutter -edx-lint diff --git a/cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile b/cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile index f44e068d..53a3c7be 100644 --- a/cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile +++ b/cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile @@ -32,6 +32,10 @@ clean: ## delete generated byte code and coverage reports rm -rf assets rm -rf pii_report +docs: ## generate Sphinx HTML documentation, including API docs + tox -e docs + $(BROWSER)docs/_build/html/index.html + piptools: ## install pinned version of pip-compile and pip-sync pip install -r requirements/pip.txt pip install -r requirements/pip-tools.txt diff --git a/cookiecutter-django-ida/{{cookiecutter.repo_name}}/{{cookiecutter.project_name}}/settings/devstack.py b/cookiecutter-django-ida/{{cookiecutter.repo_name}}/{{cookiecutter.project_name}}/settings/devstack.py index 7be83234..b648c424 100644 --- a/cookiecutter-django-ida/{{cookiecutter.repo_name}}/{{cookiecutter.project_name}}/settings/devstack.py +++ b/cookiecutter-django-ida/{{cookiecutter.repo_name}}/{{cookiecutter.project_name}}/settings/devstack.py @@ -28,19 +28,25 @@ # OAuth2 variables specific to backend service API calls. BACKEND_SERVICE_EDX_OAUTH2_KEY = os.environ.get('BACKEND_SERVICE_EDX_OAUTH2_KEY', '{{cookiecutter.project_name}}-backend-service-key') -BACKEND_SERVICE_EDX_OAUTH2_SECRET = os.environ.get('BACKEND_SERVICE_EDX_OAUTH2_SECRET', '{{cookiecutter.project_name}}-backend-service-secret') +BACKEND_SERVICE_EDX_OAUTH2_SECRET = os.environ.get('BACKEND_SERVICE_EDX_OAUTH2_SECRET', '{{cookiecutter.project_name}}-backend-service-secret') # noqa: E501 JWT_AUTH.update({ 'JWT_SECRET_KEY': 'lms-secret', 'JWT_ISSUER': 'http://localhost:18000/oauth2', 'JWT_AUDIENCE': None, 'JWT_VERIFY_AUDIENCE': False, - 'JWT_PUBLIC_SIGNING_JWK_SET': ( - '{"keys": [{"kid": "devstack_key", "e": "AQAB", "kty": "RSA", "n": "smKFSYowG6nNUAdeqH1jQQnH1PmIHphzBmwJ5vRf1vu' - '48BUI5VcVtUWIPqzRK_LDSlZYh9D0YFL0ZTxIrlb6Tn3Xz7pYvpIAeYuQv3_H5p8tbz7Fb8r63c1828wXPITVTv8f7oxx5W3lFFgpFAyYMmROC' - '4Ee9qG5T38LFe8_oAuFCEntimWxN9F3P-FJQy43TL7wG54WodgiM0EgzkeLr5K6cDnyckWjTuZbWI-4ffcTgTZsL_Kq1owa_J2ngEfxMCObnzG' - 'y5ZLcTUomo4rZLjghVpq6KZxfS6I1Vz79ZsMVUWEdXOYePCKKsrQG20ogQEkmTf9FT_SouC6jPcHLXw"}]}' - ), + 'JWT_PUBLIC_SIGNING_JWK_SET': """ + { + "keys": [ + { + "kty": "RSA", + "kid": "devstack_key", + "n": "smKFSYowG6nNUAdeqH1jQQnH1PmIHphzBmwJ5vRf1vu48BUI5VcVtUWIPqzRK_LDSlZYh9D0YFL0ZTxIrlb6Tn3Xz7pYvpIAeYuQv3_H5p8tbz7Fb8r63c1828wXPITVTv8f7oxx5W3lFFgpFAyYMmROC4Ee9qG5T38LFe8_oAuFCEntimWxN9F3P-FJQy43TL7wG54WodgiM0EgzkeLr5K6cDnyckWjTuZbWI-4ffcTgTZsL_Kq1owa_J2ngEfxMCObnzGy5ZLcTUomo4rZLjghVpq6KZxfS6I1Vz79ZsMVUWEdXOYePCKKsrQG20ogQEkmTf9FT_SouC6jPcHLXw", + "e": "AQAB" + } + ] + } + """, # noqa: E501 'JWT_ISSUERS': [{ 'AUDIENCE': 'lms-key', 'ISSUER': 'http://localhost:18000/oauth2', diff --git a/cookiecutter-xblock/openedx.yaml b/cookiecutter-xblock/openedx.yaml deleted file mode 100644 index 069a1493..00000000 --- a/cookiecutter-xblock/openedx.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# This file describes this Open edX repo, as described in OEP-2: -# https://open-edx-proposals.readthedocs.io/en/latest/oep-0002-bp-repo-metadata.html#specification - -oeps: - oep-2: true - oep-7: false -tags: - - xblock