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
feat: adding separate bucket based on python version for integration tests. #1828
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,7 +14,25 @@ class PackageIntegBase(TestCase): | |
@classmethod | ||
def setUpClass(cls): | ||
cls.region_name = os.environ.get("AWS_DEFAULT_REGION") | ||
cls.pre_created_bucket = os.environ.get("AWS_S3", False) | ||
""" | ||
Our integration tests use S3 bucket to run several tests. Given that S3 objects are eventually consistent | ||
and we are using same bucket for lot of integration tests, we want to have multiple buckets to reduce | ||
transient failures. In order to achieve this we created 3 buckets one for each python version we support (3.6, | ||
3.7 and 3.8). Tests running for respective python version will use respective bucket. | ||
|
||
AWS_S3 will point to a new environment variable AWS_S3_36 or AWS_S3_37 or AWS_S3_38. This is controlled by | ||
Appveyor. These environment variables will hold bucket name to run integration tests. Eg: | ||
|
||
For Python36: | ||
AWS_S3=AWS_S3_36 | ||
AWS_S3_36=aws-sam-cli-canary-region-awssamclitestbucket-forpython36 | ||
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. Do no document bucket names. 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. This is not a real bucket name. Something I made up based on how real name looks like. |
||
|
||
For backwards compatibility we are falling back to reading AWS_S3 so that current tests keep working. | ||
""" | ||
cls.pre_created_bucket = os.environ.get( | ||
os.environ.get("AWS_S3", False), # Reading env var key for specific bucket. | ||
os.environ.get("AWS_S3"), # Fallback to old logic if separate env key not present. | ||
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. This is not the same fallback. The previous fallback had a value of False. |
||
) | ||
cls.bucket_name = cls.pre_created_bucket if cls.pre_created_bucket else str(uuid.uuid4()) | ||
cls.test_data_path = Path(__file__).resolve().parents[1].joinpath("testdata", "package") | ||
|
||
|
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 should be below doc strings.
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.
Actually doc string is only for the line where we are reading
AWS_S3
and not for the whole method. That is why I didn't move this below docstring. Thoughts?