[Issue #3553] Modify local s3 setup in factories to be easier to use #3574
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chouinar
requested review from
mikehgrantsgov and
babebe
and removed request for
mdragon
babebe
reviewed
Jan 17, 2025
babebe
reviewed
Jan 17, 2025
| @@ -373,7 +349,7 @@ def mock_s3(reset_aws_env_vars): | |||
|
|
|||
| @pytest.fixture | |||
| def mock_s3_bucket_resource(mock_s3): | |||
| bucket = mock_s3.Bucket("test_bucket") | |||
| bucket = mock_s3.Bucket("local-mock-public-bucket") | |||
There was a problem hiding this comment.
description here for when to use one vs the other would help.
There was a problem hiding this comment.
You're free to use either however you see fit - there's not any real specific guidance. I can add a note to the other one to say that it exists because in some cases we want to test across buckets.
babebe
reviewed
Jan 17, 2025
babebe
reviewed
Jan 17, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3553
Time to review: 10 mins
Changes proposed
Modified our attachment factory to create files on s3 automatically
Context for reviewers
We had a lot of tests and local setup assume files would already be pre-generated that the opportunity attachment table would point to. Those often were wrong and broke.
This fixes things by instead making the factories create s3 files for you. It does this by taking the path you gave it, and making a file at that location. This does make assumptions that your s3 bucket exists, but I piggybacked on the
enable_factory_createpytest fixture we have that already requires you call it to call any DB operation with create, so that shouldn't be too much of an issue. I've set it up so the factory by default now creates no attachments on an opportunity, but can be told to add a few because the performance degradation on running tests was pretty bad.Additional information
Running
make db-seed-localworks. You can see the files it created withaws --endpoint-url http://localhost:4566 s3 ls local-mock-public-bucket --recursiveThe tests all work with just a few minor fixes.