-
Notifications
You must be signed in to change notification settings - Fork 71
Set the test suite regardless of actual cloud connection. #218
base: master
Are you sure you want to change the base?
Conversation
Code Climate has analyzed commit e77d23b and detected 0 issues on this pull request. View more on Code Climate. |
|
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.
I'm not sure this PR gets it all the way? The goal here should be to remove all distinctions between "live" and "non-live" tests, so that the entire test suite can run without making any real connections to backend services.
I'm not seeing anything that verifies that connections to GCP are in correct shapes, we need to have that in place so that we can safely refactor that code. That currently relies on the "live" tests failing on real connections to GCP.
The Github Actions workflow needs to be updated to remove the SKIP_LIVE_TESTS
conditionals, and probably all traces of SKIP_LIVE_TESTS
and the @live_test
should be removed too.
collectfast/tests/settings.py
Outdated
AWS_S3_SIGNATURE_VERSION = "s3v4" | ||
AWS_QUERYSTRING_AUTH = False | ||
AWS_DEFAULT_ACL = None | ||
S3_USE_SIGV4 = True | ||
AWS_S3_HOST = "s3.eu-central-1.amazonaws.com" | ||
AWS_S3_HOST = "s3.ap-northeast-2.amazonaws.com" |
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.
Is there any reason to change this?
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.
Well, just for live testing with my accounts.
I'll revert it.
@@ -3,14 +3,13 @@ | |||
|
|||
from django.test import override_settings as override_django_settings | |||
|
|||
from collectfast.tests.command.utils import call_collectstatic |
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.
This doesn't seem like a justified change, let's keep this a relative import.
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.
I thought the absolute path was more straightforward. But in this case, I guess relative is enough. 😄
@@ -5,6 +5,7 @@ | |||
from django.test import override_settings as override_django_settings | |||
|
|||
from collectfast.management.commands.collectstatic import Command | |||
from collectfast.tests.command.utils import call_collectstatic |
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.
Same here, let's restore this import.
collectfast/tests/utils.py
Outdated
import boto3 # type: ignore | ||
import moto # type: ignore |
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.
Let's install types for boto3, and ignore moto in setup.cfg.
Related: pytest should be possible to remove from setup.cfg since it has gained typing support.
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.
Sounds great.
live_test function has been deleted. SKIP_LIVE_TESTS not supported anymore.
Now we can run the test suite independently without the actual access key of AWS and GCP.
moto