Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Remove unused files; add more tests; fix some Make targets and lint #332

Merged
merged 6 commits into from
May 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
timmc-edx marked this conversation as resolved.
Show resolved Hide resolved

- 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might also add "removed unused files in django-ida, django-app, and xblock"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added something like this


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
rgraber marked this conversation as resolved.
Show resolved Hide resolved

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.