Skip to content

Commit

Permalink
fix: Remove unused files; add more tests; fix some Make targets and l…
Browse files Browse the repository at this point in the history
…int (#332)

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, although it doesn't quite work yet;
  filed #339 for fixing
  this later. (Excluded from testing for now.)
- Add some long-line lint suppression where needed

Other improvements:

- Remove unused `BAKE_OPTIONS` from root Makefile
- Use short version of `BROWSER` script in django-ida Makefile to
  match other Makefiles
- 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)
- Made `fake_translations` Makefile target actually do something.
  • Loading branch information
timmc-edx authored May 16, 2023
1 parent fe06e3e commit 06f6914
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 101 deletions.
60 changes: 60 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,60 @@ 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 validate
- name: "Ensure translations can be compiled"
working-directory: repo_name
run: |
source .venv/bin/activate
sudo apt install gettext # required for msguniq, called by makemessages management command
make fake_translations
- name: "Ensure docker can build a container"
working-directory: repo_name
run: |
source .venv/bin/activate
docker build . --target app
19 changes: 19 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,25 @@ Change Log
This file loosely adheres to the structure of https://keepachangelog.com/,
but in reStructuredText instead of Markdown.
2023-05-16
**********

Fixed
=====

- Add missing ``docs`` and ``fake_translations`` Makefile targets to ``cookiecutter-django-ida`` and suppressed long-line lint as appropriate. (The docs target is still partly broken, though.)
- Removed unused and distracting files in various cookiecutters (no effect on output)

Changed
=======

- Use short version of ``BROWSER`` script in django-ida Makefile to match others

Added
=====

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

2023-05-04
**********

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.

23 changes: 9 additions & 14 deletions cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.DEFAULT_GOAL := help

.PHONY: help clean requirements ci_requirements dev_requirements \
.PHONY: help clean docs requirements ci_requirements dev_requirements \
validation_requirements doc_requirements prod_requirements static shell \
test coverage isort_check isort style lint quality pii_check validate \
migrate html_coverage upgrade extract_translation dummy_translations \
Expand All @@ -9,17 +9,8 @@
detect_changed_source_translations validate_translations check_keywords \
install_transifex_client

define BROWSER_PYSCRIPT
import os, webbrowser, sys
try:
from urllib import pathname2url
except:
from urllib.request import pathname2url

webbrowser.open("file://" + pathname2url(os.path.abspath(sys.argv[1])))
endef
export BROWSER_PYSCRIPT
BROWSER := python -c "$$BROWSER_PYSCRIPT"
# For opening files in a browser. Use like: $(BROWSER)relative/path/to/file.html
BROWSER := python -m webbrowser file://$(CURDIR)/

# Generates a help message. Borrowed from https://github.com/pydanny/cookiecutter-djangopackage.
help: ## display this help message
Expand All @@ -32,6 +23,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 Expand Up @@ -64,7 +59,7 @@ test: clean ## run tests and generate coverage report
# To be run from CI context
coverage: clean
pytest --cov-report html
$(BROWSER) htmlcov/index.html
$(BROWSER)htmlcov/index.html

isort_check: ## check that isort has been run
isort --check-only {{cookiecutter.project_name}}/
Expand Down Expand Up @@ -130,7 +125,7 @@ dummy_translations: ## generate dummy translation (.po) files
compile_translations: # compile translation files, outputting .po files for each supported language
python manage.py compilemessages

fake_translations: ## generate and compile dummy translation files
fake_translations: extract_translations dummy_translations compile_translations ## generate and compile dummy translation files

pull_translations: ## pull translations from Transifex
tx pull -af -t --mode reviewed
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 06f6914

Please sign in to comment.