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

Convert manual integration tests to pytest tests #148

Merged
merged 4 commits into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from 2 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
5 changes: 4 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,7 @@ dmypy.json
.DS_Store

# VSCode
.vscode/
.vscode/

# PyCharm
.idea/
7 changes: 5 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ update: install # update all python dependencies
pipenv update --dev

## ---- Test commands ---- ##
test: # run tests and print coverage report
pipenv run coverage run --source=lambdas -m pytest -vv
test: # Run tests and print a coverage report
pipenv run coverage run --source=lambdas -m pytest -vv -m "not integration"
pipenv run coverage report -m

coveralls: test
pipenv run coverage lcov -o ./coverage/lcov.info

test-integration:
pipenv run pytest -vv -s -m "integration"
jonavellecuerdo marked this conversation as resolved.
Show resolved Hide resolved

## ---- Code quality and safety commands ###

# linting commands
Expand Down
70 changes: 20 additions & 50 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,61 +29,31 @@ Required env variables:
- `WORKSPACE=dev`: env for local development, set by Terraform in AWS environments.
- `SENTRY_DSN`: only needed in production.

### To verify local changes in Dev1
### Integration Tests for Dev1

- Ensure your aws cli is configured with credentials for the Dev1 account.
- Ensure you have the above env variables set in your .env, matching those in our Dev1 environment.
- Add the following to your .env: `LAMBDA_FUNCTION_URL=<the Dev1 lambda function URL>`
- Publish the lambda function:
Some minimal integration tests are provided for checking the deployed webhook handling lambda, defined as `integration` type pytest tests. These tests check the following:
* lambda function URL is operational
* `GET` and `POST` requests are received
* deployed lambda has adequate permissions to communicate with S3, StepFunctions, etc.

```bash
make publish-dev
make update-lambda-dev
```
Other notes about tests:
* tests are limited to `Dev1` environment
* AWS `AWSAdministratorAccess` role credentials must be set on developer machine
* environment variable `WORKSPACE=dev` must be set
* no other environment variables are required, these are all retrieved from deployed context

#### GET request example
#### Steps to run integration tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use numbering to enumerate these steps? I think it would help with the readability! :)


- Send a GET request with challenge phrase to the lambda function URL:

```bash
pipenv run python -c "from lambdas.helpers import send_get_to_lambda_function_url; print(send_get_to_lambda_function_url('your challenge phrase'))"
```

Observe output: `your challenge phrase`

#### POST request examples

- Send a POST request mimicking a webhook POST (not a POD or TIMDEX export job)

```bash
pipenv run python -c "from lambdas.helpers import send_post_to_lambda_function_url, SAMPLE_WEBHOOK_POST_BODY; print(send_post_to_lambda_function_url(SAMPLE_WEBHOOK_POST_BODY))"
```

Observe output: `Webhook POST request received and validated, no action taken.`

- Send a POST request mimicking a POD export job webhook
_Note_: sending a request that mimics a POD export JOB_END will trigger the entire POD workflow, which is fine _in Dev1 only_ for testing.

Add the following to your .env:
- `VALID_POD_EXPORT_DATE=<the date of a POD export with files in the Dev1 S3 export bucket, in "YYYY-MM-DD" format>` Note: if it's been a while since the last POD export from Alma sandbox, there may be no files in the Dev1 S3 export bucket and you may need to run the publishing job from the sandbox.

```bash
pipenv run python -c "from lambdas.helpers import send_post_to_lambda_function_url, SAMPLE_POD_EXPORT_JOB_END_WEBHOOK_POST_BODY; print(send_post_to_lambda_function_url(SAMPLE_POD_EXPORT_JOB_END_WEBHOOK_POST_BODY))"
```

Observe output: `Webhook POST request received and validated, PPOD pipeline initiated.` and then check the Dev1 ppod state machine logs to confirm the entire process ran!

- Send a POST request mimicking a TIMDEX export job webhook
_Note_: sending a request that mimics a TIMDEX export JOB_END will trigger the entire TIMDEX workflow, which is fine _in Dev1 only_ for testing.

Add the following to your .env:
- `VALID_TIMDEX_EXPORT_DATE=<the date of a TIMDEX export with files in the Dev1 S3 export bucket, in "YYYY-MM-DD" format>` Note: if it's been a while since the last TIMDEX export from Alma sandbox, there may be no files in the Dev1 S3 export bucket and you may need to run the publishing job from the sandbox.

```bash
pipenv run python -c "from lambdas.helpers import send_post_to_lambda_function_url, SAMPLE_TIMDEX_EXPORT_JOB_END_WEBHOOK_POST_BODY; print(send_post_to_lambda_function_url(SAMPLE_TIMDEX_EXPORT_JOB_END_WEBHOOK_POST_BODY))"
```
Update docker image to ensure local changes are deployed to `Dev1`:
```shell
make publish-dev
make update-lambda-dev
```

Observe output: `Webhook POST request received and validated, TIMDEX pipeline initiated.` and then check the Dev1 timdex state machine logs to confirm the entire process ran!
Run tests against deployed assets:
```shell
make test-integration
```

## Running locally with Docker
jonavellecuerdo marked this conversation as resolved.
Show resolved Hide resolved

Expand Down
37 changes: 0 additions & 37 deletions lambdas/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,43 +5,6 @@

import requests

SAMPLE_WEBHOOK_POST_BODY = {
"action": "JOB_END",
"job_instance": {
"name": "Not a POD export job",
},
}

SAMPLE_POD_EXPORT_JOB_END_WEBHOOK_POST_BODY = {
"action": "JOB_END",
"job_instance": {
"name": os.getenv("ALMA_POD_EXPORT_JOB_NAME", "PPOD Export"),
"end_time": os.getenv("VALID_POD_EXPORT_DATE", "2022-05-23"),
"status": {"value": "COMPLETED_SUCCESS"},
"counter": [
{
"type": {"value": "label.new.records", "desc": "New Records"},
"value": "1",
},
],
},
}

SAMPLE_TIMDEX_EXPORT_JOB_END_WEBHOOK_POST_BODY = {
"action": "JOB_END",
"job_instance": {
"name": "Publishing Platform Job TIMDEX EXPORT to Dev1 DAILY",
"status": {"value": "COMPLETED_SUCCESS"},
"end_time": os.getenv("VALID_TIMDEX_EXPORT_DATE", "2022-10-24"),
"counter": [
{
"type": {"value": "label.new.records", "desc": "New Records"},
"value": "1",
},
],
},
}


def generate_signature(message_body: dict) -> str:
secret = os.environ["ALMA_CHALLENGE_SECRET"]
Expand Down
7 changes: 6 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ exclude = ["tests/"]

[tool.pytest.ini_options]
log_level = "INFO"
markers = [
"unit: unit tests",
"integration: integration tests"
]

[tool.ruff]
target-version = "py311"
Expand All @@ -25,7 +29,8 @@ ignore = [
# project-specific
"D100",
"D103",
"D104"
"D104",
"G002"
]

# allow autofix behavior for specified rules
Expand Down
180 changes: 180 additions & 0 deletions tests/conftest.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,23 @@
# ruff: noqa: G004
import datetime
import logging
import os
import urllib
from importlib import reload

import boto3
import botocore.session
import pytest
import requests_mock
from botocore.exceptions import ClientError
from botocore.stub import Stubber

import lambdas.helpers
import lambdas.webhook
from lambdas.webhook import lambda_handler

ORIGINAL_ENV = os.environ.copy()


@pytest.fixture(autouse=True)
def _test_env():
Expand Down Expand Up @@ -154,3 +161,176 @@ def stubbed_bursar_sfn_client():
with Stubber(sfn) as stubber:
stubber.add_response("start_execution", expected_response, expected_params)
yield sfn


@pytest.fixture
def _set_integration_tests_env_vars() -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

While the comments help, this is a bit involved and long so you might consider breaking this up into smaller fixtures that are then called by this fixture for readability

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's a good suggestion. This prompted utilizing a contextmanager to temporarily reset the environment, for each integration test, which gaurantees it's torn down correctly as well.

Also broke the lambda config and SMS retrievals into their own functions. Hopefully you can look at this fixture now (changes coming in a new commit) and see that it's:

  1. using a context manager to temporarily reset the environment
  2. call some functions that get data from AWS and set to environment
  3. release all those changes when that test completes

"""Fixture to set os environment variables by retrieving data from AWS.

Because mocked AWS credentials are set in the testing environment, this temporary
reinstating of the calling environment (e.g. developer's machine) when the tests
began is required. Once data about deployed assets, e.g. lambda function URL and
deployed environment variables, is retrieved, the testing environment is used again.
"""
# backup current, test env
test_env = os.environ.copy()

# set os.environ as original env before testing framework
# ruff: noqa: B003
os.environ = ORIGINAL_ENV

try:
if not os.getenv("WORKSPACE"):
# ruff: noqa: TRY301, TRY002, TRY003, EM101
raise Exception("WORKSPACE env var must be set for integration tests")

# get lambda configurations
lambda_client = boto3.client("lambda")
lambda_function_config = lambda_client.get_function_configuration(
FunctionName=f"alma-webhook-lambdas-{os.getenv('WORKSPACE').lower()}"
ghukill marked this conversation as resolved.
Show resolved Hide resolved
)
lambda_function_env_vars = lambda_function_config["Environment"]["Variables"]
lambda_function_url = lambda_client.get_function_url_config(
FunctionName=f"alma-webhook-lambdas-{os.getenv('WORKSPACE').lower()}"
)["FunctionUrl"]

# get values from parameter store
ssm_client = boto3.client("ssm")
ssm_client.get_parameter(Name="/apps/almahook/alma-pod-export-job-name")[
"Parameter"
]["Value"]
ppod_state_machine_arn = ssm_client.get_parameter(
Name="/apps/almahook/ppod-state-machine-arn"
)["Parameter"]["Value"]
timdex_state_machine_arn = ssm_client.get_parameter(
Name="/apps/almahook/timdex-ingest-state-machine-arn"
)["Parameter"]["Value"]

except:
logging.exception("could not retrieve lambda configurations via boto3")
ghukill marked this conversation as resolved.
Show resolved Hide resolved
raise
finally:
# reset testing env vars
os.environ = test_env

# set env vars
os.environ["LAMBDA_FUNCTION_URL"] = lambda_function_url
os.environ["ALMA_CHALLENGE_SECRET"] = lambda_function_env_vars[
"ALMA_CHALLENGE_SECRET"
]
os.environ["ALMA_POD_EXPORT_JOB_NAME"] = "Publishing Platform Job PPOD EXPORT to Dev1"
os.environ["PPOD_STATE_MACHINE_ARN"] = ppod_state_machine_arn
os.environ["VALID_POD_EXPORT_DATE"] = "2023-08-15" # matches fixture date
os.environ["TIMDEX_STATE_MACHINE_ARN"] = timdex_state_machine_arn
os.environ["VALID_TIMDEX_EXPORT_DATE"] = "2023-08-15" # matches fixture date


@pytest.fixture
def _integration_tests_s3_fixtures(_set_integration_tests_env_vars) -> None:
"""Upload integration test fixtures to S3, if they don't already exist.

These s3 files are used by deployed assets during integration tests. This fixture
relies on _set_integration_tests_env_vars as a dependency to ensure AWS credentials
have not been clobbered by testing env vars.
"""
s3 = boto3.client("s3")

def check_and_upload_file(bucket, key):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure of the best practice on something like this but this design pattern feels weird (defining a function inside a fixture). It seems possible to just stick this at the top:

# Specify your bucket and key
    fixtures = [
        (
            "dev-sftp-shared",
            "exlibris/pod/POD_ALMA_EXPORT_20230815_220844[016]_new.tar.gz",
        ),
        (
            "dev-sftp-shared",
            "exlibris/timdex/TIMDEX_ALMA_EXPORT_DAILY_20230815_220844[016]_new.tar.gz",
        ),
    ]
    for bucket, key in fixtures:

But I don't have enough experience with this type of integration tests to have much confidence in commenting on this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! Much easier to call via a loop, and can tighten up that S3 file check/upload code as well.

try:
s3.head_object(Bucket=bucket, Key=key + "foo")
logging.info(f"File s3://{bucket}/{key} already exists, nothing to do!")
except ClientError as e:
error_code = int(e.response["Error"]["Code"])
# ruff: noqa: PLR2004
if error_code == 404:
logging.info(f"File s3://{bucket}/{key} not found. Uploading...")
local_file_path = os.path.join("tests", "fixtures", os.path.basename(key))
if os.path.exists(local_file_path):
s3.upload_file(local_file_path, bucket, key)
logging.info(f"File uploaded to s3://{bucket}/{key}")
else:
msg = f"Fixture file {local_file_path} does not exist."
logging.exception(msg)
raise FileNotFoundError(msg) from None
else:
raise

# Specify your bucket and key
fixtures = [
(
ghukill marked this conversation as resolved.
Show resolved Hide resolved
"dev-sftp-shared",
"exlibris/pod/POD_ALMA_EXPORT_20230815_220844[016]_new.tar.gz",
),
(
"dev-sftp-shared",
"exlibris/timdex/TIMDEX_ALMA_EXPORT_DAILY_20230815_220844[016]_new.tar.gz",
),
]
for bucket, key in fixtures:
check_and_upload_file(bucket, key)


@pytest.fixture
def sample_webhook_post_body() -> dict:
return {
"action": "JOB_END",
"job_instance": {
"name": "Not a POD export job",
},
}


@pytest.fixture
def sample_pod_export_job_end_webhook_post_body() -> dict:
return {
"action": "JOB_END",
"job_instance": {
"name": os.getenv("ALMA_POD_EXPORT_JOB_NAME", "PPOD Export"),
"end_time": os.getenv("VALID_POD_EXPORT_DATE", "2022-05-23"),
"status": {"value": "COMPLETED_SUCCESS"},
"counter": [
{
"type": {"value": "label.new.records", "desc": "New Records"},
"value": "1",
},
],
},
}


@pytest.fixture
def sample_timdex_export_job_end_webhook_post_body() -> dict:
return {
"action": "JOB_END",
"job_instance": {
"name": "Publishing Platform Job TIMDEX EXPORT to Dev1 DAILY",
"status": {"value": "COMPLETED_SUCCESS"},
"end_time": os.getenv("VALID_TIMDEX_EXPORT_DATE", "2022-10-24"),
"counter": [
{
"type": {"value": "label.new.records", "desc": "New Records"},
"value": "1",
},
],
},
}


def pytest_collection_modifyitems(config, items):
"""Hook that is run after all tests collected, which allows for modification pre-run.

https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.hookspec.pytest_collection_modifyitems
"""
# skip integration tests if WORKSPACE is not 'dev'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd move this into the docstring, I like this function

Copy link
Contributor Author

@ghukill ghukill Sep 1, 2023

Choose a reason for hiding this comment

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

Normally I'd agree, but this hook is a built-in function for pytests, and is called just by virtue of being named pytest_collection_modifyitems. Therefore, if we want to add other logic that should fire during this hook, it would also get added here.

But now that you mention it.. this behavior could be put into another function that this hook calls. Then, that function's docstring can explain what it does, and future logic / functions could be added to this hook if needed.

Thanks for flagging!

allowed_test_environments = ["dev"]
for item in items:
if (
item.get_closest_marker("integration")
and os.getenv("WORKSPACE") not in allowed_test_environments
):
item.add_marker(
pytest.mark.skip(
reason="integration tests currently only support environments: %s"
% allowed_test_environments
)
)
Binary file not shown.
Binary file not shown.
Empty file added tests/integration/__init__.py
Empty file.
Loading
Loading