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

Add testing and remove unused code #257

Merged
merged 38 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
2d5ca67
Install Playwright
symroe Nov 12, 2024
7ad6b10
Test frontend
symroe Nov 12, 2024
6217477
Handle cases where no initial date exists
VirginiaDooley Oct 7, 2024
a0508de
Use YNR auth token to address rate limiting
VirginiaDooley Oct 7, 2024
395ea78
Test redirect after leaflet creation
VirginiaDooley Oct 9, 2024
456fca4
Install and use pytest
VirginiaDooley Oct 9, 2024
4a8e3f5
Convert tests to use pytest
VirginiaDooley Oct 9, 2024
abd4b4a
Convert image_editing tests from TestCase to pytest
VirginiaDooley Oct 10, 2024
871fda9
Remove unused code
VirginiaDooley Oct 10, 2024
9da376a
Return a relative path to address path traversal attempt
VirginiaDooley Oct 10, 2024
5f91235
Saving an LeafletImage requires an image file and name
VirginiaDooley Oct 10, 2024
b46d2cc
Adjust tests after asking pytest to use testing.py
VirginiaDooley Oct 10, 2024
49fe17a
Add pytest coverage for core forms
VirginiaDooley Oct 10, 2024
5e1b270
Add pytest coverage for core models
VirginiaDooley Oct 10, 2024
703d9e6
Remove bulk of analysis app
symroe Nov 12, 2024
767b789
Add pytest coverage for api views
VirginiaDooley Oct 14, 2024
85bdf20
Add pytest coverage for api feeds
VirginiaDooley Oct 14, 2024
6e56553
Update deploy instructions
VirginiaDooley Oct 15, 2024
772e36d
Handle multiple parties returned
VirginiaDooley Oct 15, 2024
146218c
Convert Person model tests from TestCase to pytest
VirginiaDooley Oct 15, 2024
9362d00
Fix status message
VirginiaDooley Oct 16, 2024
3042d04
Remove codecov arguements; TODO: config for codecov
VirginiaDooley Oct 16, 2024
a6c6408
Clear cache
VirginiaDooley Oct 17, 2024
cb51f0a
Remove six; no longer used
VirginiaDooley Oct 17, 2024
6844a8b
Use live_server from pytest-django to start server
VirginiaDooley Oct 17, 2024
961fb3b
Remove bulk of constituencies app
symroe Nov 12, 2024
a1305e7
Remove party importer
symroe Nov 15, 2024
8bfa638
DRY up YNR base URL
symroe Nov 15, 2024
426786a
Import switcher in design system
symroe Nov 19, 2024
c8bb940
Mock YNR in tests
symroe Nov 19, 2024
6e1e352
Don't add Party objects when saving leaflets
symroe Nov 19, 2024
7d3d0a6
Fail tests if errors are written to the browser console
symroe Nov 20, 2024
246443d
USe test settings in CI and ensure static works
symroe Nov 20, 2024
e4695fa
Save images depending on the backend
symroe Nov 20, 2024
fa7264e
Don't set the default postgres user in base settings
symroe Nov 21, 2024
29cac4c
Remove test_media and use tempdir as MEDIA_ROOT in tests
symroe Nov 25, 2024
2da7fd9
Fix Pipfile test command
symroe Nov 27, 2024
64c67b1
Use debug mode for live_server
symroe Nov 27, 2024
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
30 changes: 15 additions & 15 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ jobs:
- checkout
- restore_cache:
keys:
- v7-dependencies-{{ checksum "Pipfile.lock" }}-{{ checksum "thumbs/Pipfile.lock" }}
- v7-dependencies-default
- v8-dependencies-{{ checksum "Pipfile.lock" }}-{{ checksum "thumbs/Pipfile.lock" }}
- v8-dependencies-default
# Ensure pip and pipenv are installed
- run:
name: Install pipenv
Expand All @@ -24,43 +24,42 @@ jobs:
name: Set up virtual environment
command: |
pipenv sync --dev
- run: pipenv run playwright install --with-deps chromium
- run: make lambda-layers/DependenciesLayer/requirements.txt
- run: pipenv run collectstatic
- run: npm install
- run: npm run build
- run: DJANGO_SETTINGS_MODULE=electionleaflets.settings.base_lambda pipenv run collectstatic
- run: pipenv run collectstatic --settings electionleaflets.settings.testing
- persist_to_workspace:
root: ~/repo/
paths:
- .aws-sam/build/
- thumbs/.aws-sam/build/
- lambda-layers/DependenciesLayer/requirements.txt
- electionleaflets/static/
- electionleaflets/assets/
- save_cache:
when: on_success
paths:
- ~/.local/share/virtualenvs/
key: v7-dependencies-{{ checksum "Pipfile.lock" }}-{{ checksum "thumbs/Pipfile.lock" }}
key: v8-dependencies-{{ checksum "Pipfile.lock" }}-{{ checksum "thumbs/Pipfile.lock" }}
run-tests:
docker:
- image: cimg/python:3.12
- image: cimg/python:3.12-browsers
environment:
CIRCLECI: true
PGHOST: 127.0.0.1
- image: cimg/postgres:16.3
environment:
POSTGRES_USER: electionleaflets
POSTGRES_USER: circleci
POSTGRES_DB: electionleaflets
working_directory: ~/repo

steps:
- checkout

- attach_workspace:
at: ~/repo/
# Download and cache dependencies
- restore_cache:
keys:
- v7-dependencies-{{ checksum "Pipfile.lock" }}

- v8-dependencies-{{ checksum "Pipfile.lock" }}
- run: pipenv run pipenv verify

- run: pipenv run python manage.py check
Expand All @@ -69,10 +68,11 @@ jobs:
#- run: pipenv run python manage.py makemigrations --check

# Run tests
- run: pipenv run playwright install --with-deps chromium
- run:
name: Run tests
command: |
pipenv run test
pipenv run pytest
- store_artifacts:
path: test-results
destination: test-results
Expand Down Expand Up @@ -125,8 +125,8 @@ jobs:
at: ~/repo/
- restore_cache:
keys:
- v7-dependencies-{{ checksum "Pipfile.lock" }}
- v7-dependencies-default
- v8-dependencies-{{ checksum "Pipfile.lock" }}
- v8-dependencies-default
- run:
name: Install pipenv
command: |
Expand Down
11 changes: 8 additions & 3 deletions Pipfile
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ enum34 = "==1.1.6"
fixtures = "==3.0.0"
flake8 = "==3.8.3"
funcsigs = "==1.0.2"
futures = "==3.1.1"
dj-pagination = "==2.4.0"
linecache2 = "==1.0.0"
lxml = "*"
Expand Down Expand Up @@ -80,15 +79,21 @@ flask = "==2.3.2"
certifi = "*"
Markdown = "*"
dc-django-utils = {file = "https://github.com/DemocracyClub/dc_django_utils/archive/refs/tags/7.0.1.tar.gz"}
playwright = "==1.48.0"
pytest-django = "*"
pytest = "*"
pytest-playwright = "*"
pytest-cov = "*"
moto = "*"
Copy link
Member

Choose a reason for hiding this comment

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

One of the things I was hoping we would cover under the banner of "removing unused code[/stuff]" was unused packages. I think there are 2 big problems we have on this project:

  1. I am convinced there is a bunch of stuff in the package tree that we actually don't use at all any more
  2. There is no seperation of prod and dev packages. This is especially problematic for an application deployed to lambda because every time we do a cold boot we are spending time unzipping testing/dev packages that are not used at runtime. Specifically here you are adding playwright, pytest, moto etc into the stuff we have to unzip when we cold boot.

I'd really like us to fix both of these problems.
If the next thing you are going to do is migrate from pipenv to UV, I am happy to just ignore this for now and include it in the UV migration PR. This was one of the things I did as part of that exercise on EE:

Although the scale of the problem is bigger on this project.

Anyway. If we want to just ignore it for now, fine. Just want to flag it as a thing that needs doing.


[requires]
python_version = "3.12"

[scripts]
collectstatic = "python manage.py collectstatic -c --noinput"
collectstatic = "python manage.py collectstatic --noinput"
createcachetable = "python manage.py createcachetable"
coveralls = "coveralls"
migratedb = "python manage.py migrate --noinput"
makemigrations = "python manage.py makemigrations"
thumbs_sam_build = "bash -c 'cd thumbs && sam build'"
test = "bash -c 'DJANGO_SETTINGS_MODULE=electionleaflets.settings.testing python manage.py test'"
test = "pytest"
1,483 changes: 898 additions & 585 deletions Pipfile.lock

Large diffs are not rendered by default.

22 changes: 1 addition & 21 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,4 @@ The app is deployed to AWS using the [Serverless
Framework](https://serverless.com/). This manages resources in AWS Lambda and
AWS API Gateway.

To deploy:

1. Install `serverless` with `npm install -g serverless`
2. Install plugins with `npm install`
3. Deploy to `staging` with `sls deploy --stage=staging`
4. Test at https://staging.electionleaflets.org/
5. Deploy to production with `sls deploy --stage=production`

We use this now instead of Zappa for a few reasons:

- Zappa seems to now be unsupported.
- `sls` has become the industry standard and has a good plugin ecosystem.
- `sls` will manage domains and CloudFront distributions for us.

The main differences are:

- `sls` stores secrets in AWS SSM Parameter Store, instead of a JSON file in S3.
Manage these in the [Parameter Store
Console](https://eu-west-1.console.aws.amazon.com/systems-manager/parameters).
- `sls` uses API Gateway to manage CloudFront distributions. This means the
distributions will not show in the CloudFront console.
The app is deployed through CircleCI. The staging deployment is triggered by temporarily adding the current branch to context in the sam-deploy job. The production deployment is triggered by merging to master.
166 changes: 0 additions & 166 deletions electionleaflets/apps/analysis/forms.py

This file was deleted.

Loading