Skip to content

Commit

Permalink
fix: Remove unused files; add more tests; fix IDA docs and lint
Browse files Browse the repository at this point in the history
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
  #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)
  • Loading branch information
timmc-edx committed May 4, 2023
1 parent 5812349 commit 1949d81
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 87 deletions.
67 changes: 67 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ on:
branches:
- '**'

defaults:
run:
shell: bash # opts into error checking

jobs:
run_tests:
Expand Down Expand Up @@ -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
13 changes: 13 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,19 @@ Change Log
This file loosely adheres to the structure of https://keepachangelog.com/,
but in reStructuredText instead of Markdown.
2023-05-05
**********

Fixed
=====

- Added missing ``docs`` Makefile target to ``cookiecutter-django-ida`` and suppressed long-line lint as appropriate

Added
=====

- Improved testing for ``cookiecutter-django-ida`` (migrations, quality check, docker image build, docs build)

2023-05-03
**********

Expand Down
12 changes: 6 additions & 6 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
.PHONY: help quality requirements test upgrade validate

BAKE_OPTIONS=--no-input

help: ## display this help message
@echo "Please use \`make <target>' where <target> is one of"
@awk -F ':.*?## ' '/^[a-zA-Z]/ && NF==2 {printf "\033[36m %-25s\033[0m %s\n", $$1, $$2}' $(MAKEFILE_LIST) | sort
Expand Down Expand Up @@ -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
Expand Down
33 changes: 0 additions & 33 deletions cookiecutter-django-app/Makefile

This file was deleted.

31 changes: 0 additions & 31 deletions cookiecutter-django-ida/Makefile

This file was deleted.

2 changes: 0 additions & 2 deletions cookiecutter-django-ida/requirements.txt

This file was deleted.

4 changes: 4 additions & 0 deletions cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
8 changes: 0 additions & 8 deletions cookiecutter-xblock/openedx.yaml

This file was deleted.

0 comments on commit 1949d81

Please sign in to comment.