-
Notifications
You must be signed in to change notification settings - Fork 14
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
TST: Reorganizing tests and moving to pytest-style #103
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,27 @@ | ||
import pytest | ||
from aws_cdk import App | ||
from aws_cdk import App, Environment | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def account(): | ||
return "1234567890" | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def region(): | ||
return "us-east-1" | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def sds_id(): | ||
return "sdsid-test" | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def env(account, region): | ||
return Environment(account=account, region=region) | ||
|
||
|
||
@pytest.fixture(scope="module") | ||
def app(sds_id): | ||
return App(context={"SDSID": sds_id}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import os | ||
|
||
import boto3 | ||
import pytest | ||
from moto import mock_s3 | ||
|
||
|
||
@pytest.fixture() | ||
def _aws_credentials(): | ||
"""Mocked AWS Credentials for moto.""" | ||
os.environ["AWS_ACCESS_KEY_ID"] = "testing" | ||
os.environ["AWS_SECRET_ACCESS_KEY"] = "testing" | ||
os.environ["AWS_SECURITY_TOKEN"] = "testing" | ||
os.environ["AWS_SESSION_TOKEN"] = "testing" | ||
os.environ["AWS_DEFAULT_REGION"] = "us-east-1" | ||
|
||
|
||
@pytest.fixture() | ||
def s3_client(_aws_credentials): | ||
"""Mocked S3 Client, so we don't need network requests.""" | ||
with mock_s3(): | ||
yield boto3.client("s3", region_name="us-east-1") |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
from pathlib import Path | ||
|
||
import pytest | ||
|
||
from sds_data_manager.lambda_code.SDSCode.download_query_api import lambda_handler | ||
|
||
BUCKET_NAME = "test-bucket" | ||
TEST_FILE = "science_block_20221116_163611Z_idle.bin" | ||
|
||
|
||
@pytest.fixture(autouse=True) | ||
def setup_s3(s3_client): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure how much we should care about this for test functions, but it looks like some docstrings are missing here (as well as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, good catch here! I think if something isn't immediately obvious we should have a docstring or comment letting us know what it is. I've also added a few others. Specifically, though, I'd say that fixtures definitely should have some comments since they are meant for re-use, tests I don't mind as much if they don't have docstrings as long as they are named well and obvious about what they do. So, up to the developer on those. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd also like to see a specific note on whether the tests are using mocked clients or real ones - unless it's obvious. So I think the existing comment is good! |
||
"""Populate the mocked s3 client with a bucket and a file | ||
|
||
Each test below will use this fixture by default | ||
""" | ||
s3_client.create_bucket( | ||
Bucket=BUCKET_NAME, | ||
) | ||
result = s3_client.list_buckets() | ||
assert len(result["Buckets"]) == 1 | ||
assert result["Buckets"][0]["Name"] == BUCKET_NAME | ||
|
||
# upload a file | ||
local_filepath = Path(__file__).parent.parent.resolve() / f"test-data/{TEST_FILE}" | ||
s3_client.upload_file(local_filepath, BUCKET_NAME, TEST_FILE) | ||
file_list = s3_client.list_objects(Bucket=BUCKET_NAME)["Contents"] | ||
assert len(file_list) == 1 | ||
return s3_client | ||
|
||
|
||
def test_object_exists_with_s3_uri(): | ||
"""Test that this object exists within s3""" | ||
event = { | ||
"version": "2.0", | ||
"routeKey": "$default", | ||
"rawPath": "/", | ||
"rawQueryString": f"s3_uri=s3://{BUCKET_NAME}/{TEST_FILE}", | ||
"queryStringParameters": {"s3_uri": f"s3://{BUCKET_NAME}/{TEST_FILE}"}, | ||
} | ||
|
||
response = lambda_handler(event=event, context=None) | ||
assert response["statusCode"] == 200 | ||
assert "download_url" in response["body"] | ||
|
||
|
||
def test_object_exists_with_s3_uri_fails(): | ||
"""Test that objects exist in s3 fails""" | ||
event = { | ||
"version": "2.0", | ||
"routeKey": "$default", | ||
"rawPath": "/", | ||
"rawQueryString": f"s3_uri=s3://{BUCKET_NAME}/bad_path/bad_file.txt", | ||
"queryStringParameters": { | ||
"s3_uri": f"s3://{BUCKET_NAME}/bad_path/bad_file.txt" | ||
}, | ||
} | ||
|
||
response = lambda_handler(event=event, context=None) | ||
assert response["statusCode"] == 404 | ||
|
||
|
||
def test_input_parameters_missing(): | ||
"""Test that required input parameters exist""" | ||
empty_para_event = { | ||
"version": "2.0", | ||
"routeKey": "$default", | ||
"rawPath": "/", | ||
"rawQueryString": "", | ||
} | ||
|
||
bad_para_event = { | ||
"version": "2.0", | ||
"routeKey": "$default", | ||
"rawPath": "/", | ||
"rawQueryString": f"bad_input={TEST_FILE}", | ||
"queryStringParameters": {"bad_input": f"{TEST_FILE}"}, | ||
} | ||
|
||
response = lambda_handler(event=empty_para_event, context=None) | ||
assert response["statusCode"] == 400 | ||
|
||
response = lambda_handler(event=bad_para_event, context=None) | ||
assert response["statusCode"] == 400 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean all the tests in lambda_endpoints are using a mock for all the S3 calls?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, any tests that have
def test_name(s3_client):
in them would use this fixture and therefore get the mock. I also added another "autouse" fixture that relies on this one so that all of the lambda_endpoints get this during the setup step without needing to think about it. It is perhaps hiding a bit too much, but with the number of tests we had defined it seemed nicer to do this and make sure we were getting mocks all the time rather than a potential mix in the future.