From c082546aa067a67f89f4c223113fb2db20235f2d Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 12 Sep 2023 10:15:07 -0700 Subject: [PATCH 1/3] add migration for e2e tests and improve code coverage --- migrations/versions/0401_add_e2e_test_user.py | 45 ++++++++++ tests/app/aws/test_s3.py | 85 ++++++++++++++++++- tests/app/notifications/test_validators.py | 12 ++- 3 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/0401_add_e2e_test_user.py diff --git a/migrations/versions/0401_add_e2e_test_user.py b/migrations/versions/0401_add_e2e_test_user.py new file mode 100644 index 000000000..d3d9ed5e5 --- /dev/null +++ b/migrations/versions/0401_add_e2e_test_user.py @@ -0,0 +1,45 @@ +""" + +Revision ID: 0400_add_total_message_limit +Revises: 0399_remove_research_mode +Create Date: 2023-04-24 11:35:22.873930 + +""" +import os +import uuid + +from alembic import op +import sqlalchemy as sa + +from app import db +from app.dao.users_dao import get_user_by_email +from app.models import User + +revision = "0401_add_e2e_test_user" +down_revision = "0400_add_total_message_limit" + + +def upgrade(): + email_address = os.getenv("NOTIFY_E2E_TEST_EMAIL") + password = os.getenv("NOTIFY_E2E_TEST_PASSWORD") + name = f"e2e_test_user_{uuid.uuid4()}" + data = { + "id": uuid.uuid4(), + "name": name, + "email_address": email_address, + "password": password, + "mobile_number": "+12025555555", + "state": "active", + } + user = User(**data) + db.session.add(user) + db.session.commit() + + +def downgrade(): + email_address = os.getenv("NOTIFY_E2E_TEST_EMAIL") + user_to_delete = get_user_by_email(email_address) + if not user_to_delete: + return + db.session.remove(user_to_delete) + db.session.commit() diff --git a/tests/app/aws/test_s3.py b/tests/app/aws/test_s3.py index 57d7474c4..2989d86a9 100644 --- a/tests/app/aws/test_s3.py +++ b/tests/app/aws/test_s3.py @@ -1,7 +1,11 @@ +import os from datetime import datetime from os import getenv -from app.aws.s3 import get_s3_file +import pytest +from botocore.exceptions import ClientError + +from app.aws.s3 import file_exists, get_s3_file, remove_csv_object, remove_s3_object default_access_key = getenv("CSV_AWS_ACCESS_KEY_ID") default_secret_key = getenv("CSV_AWS_SECRET_ACCESS_KEY") @@ -33,3 +37,82 @@ def test_get_s3_file_makes_correct_call(notify_api, mocker): default_secret_key, default_region, ) + + +def test_remove_csv_object(notify_api, mocker): + get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") + remove_csv_object("mykey") + + get_s3_mock.assert_called_once_with( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + + +def test_remove_csv_object_alternate(notify_api, mocker): + get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") + remove_s3_object( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + + get_s3_mock.assert_called_once_with( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + + +def test_file_exists_true(notify_api, mocker): + get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") + + file_exists( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + get_s3_mock.assert_called_once_with( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + + +def test_file_exists_false(notify_api, mocker): + get_s3_mock = mocker.patch("app.aws.s3.get_s3_object") + error_response = { + "Error": {"Code": 500, "Message": "bogus"}, + "ResponseMetadata": {"HTTPStatusCode": 500}, + } + get_s3_mock.side_effect = ClientError( + error_response=error_response, operation_name="bogus" + ) + + with pytest.raises(ClientError): + file_exists( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) + + get_s3_mock.assert_called_once_with( + os.getenv("CSV_BUCKET_NAME"), + "mykey", + default_access_key, + default_secret_key, + default_region, + ) diff --git a/tests/app/notifications/test_validators.py b/tests/app/notifications/test_validators.py index dbaebf960..5ee731266 100644 --- a/tests/app/notifications/test_validators.py +++ b/tests/app/notifications/test_validators.py @@ -5,7 +5,7 @@ from notifications_utils import SMS_CHAR_COUNT_LIMIT import app from app.dao import templates_dao -from app.models import EMAIL_TYPE, SMS_TYPE +from app.models import EMAIL_TYPE, KEY_TYPE_NORMAL, SMS_TYPE from app.notifications.process_notifications import create_content_for_notification from app.notifications.sns_cert_validator import ( VALID_SNS_TOPICS, @@ -21,6 +21,7 @@ from app.notifications.validators import ( check_reply_to, check_service_email_reply_to_id, check_service_over_api_rate_limit, + check_service_over_total_message_limit, check_service_sms_sender_id, check_template_is_active, check_template_is_for_notification_type, @@ -727,3 +728,12 @@ def test_get_string_to_sign(): # This is a test payload with no valid cert, so it should raise a ValueError with pytest.raises(ValueError): validate_sns_cert(sns_payload) + + +def test_check_service_over_total_message_limit(mocker, sample_service): + get_redis_mock = mocker.patch("app.notifications.validators.redis_store.get") + get_redis_mock.return_value = None + service_stats = check_service_over_total_message_limit( + KEY_TYPE_NORMAL, sample_service + ) + assert service_stats == 0 From 75b7c6cf0592e954388e11b355741fed4cf73043 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 12 Sep 2023 11:13:09 -0700 Subject: [PATCH 2/3] add secrets to github workflow --- .github/workflows/checks.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index a0fe99391..55159c950 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -52,6 +52,10 @@ jobs: run: poetry run coverage run --omit=*/notifications_utils/* -m pytest --maxfail=10 env: SQLALCHEMY_DATABASE_TEST_URI: postgresql://user:password@localhost:5432/test_notification_api + NOTIFY_E2E_TEST_EMAIL: ${{ secrets.NOTIFY_E2E_TEST_EMAIL }} + NOTIFY_E2E_TEST_HTTP_AUTH_PASSWORD: ${{ secrets.NOTIFY_E2E_TEST_HTTP_AUTH_PASSWORD }} + NOTIFY_E2E_TEST_HTTP_AUTH_USER: ${{ secrets.NOTIFY_E2E_TEST_HTTP_AUTH_USER }} + NOTIFY_E2E_TEST_PASSWORD: ${{ secrets.NOTIFY_E2E_TEST_PASSWORD }} - name: Check coverage threshold run: poetry run coverage report --fail-under=50 From 16defbe30d469edd180418f5017942d695e21287 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 12 Sep 2023 11:29:21 -0700 Subject: [PATCH 3/3] try again --- .github/workflows/checks.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/checks.yml b/.github/workflows/checks.yml index 55159c950..dfa48c934 100644 --- a/.github/workflows/checks.yml +++ b/.github/workflows/checks.yml @@ -42,6 +42,10 @@ jobs: run: make bootstrap env: SQLALCHEMY_DATABASE_TEST_URI: postgresql://user:password@localhost:5432/test_notification_api + NOTIFY_E2E_TEST_EMAIL: ${{ secrets.NOTIFY_E2E_TEST_EMAIL }} + NOTIFY_E2E_TEST_HTTP_AUTH_PASSWORD: ${{ secrets.NOTIFY_E2E_TEST_HTTP_AUTH_PASSWORD }} + NOTIFY_E2E_TEST_HTTP_AUTH_USER: ${{ secrets.NOTIFY_E2E_TEST_HTTP_AUTH_USER }} + NOTIFY_E2E_TEST_PASSWORD: ${{ secrets.NOTIFY_E2E_TEST_PASSWORD }} - name: Run style checks run: poetry run flake8 . - name: Check imports alphabetized