FileUpload test race#1179
Merged
Merged
Conversation
AWS Local generates bucket names based on the prefix of the test name. Because these tests are so close together and relatively simple, there a chance they run in the same millisecond getting the same bucket name. So one completing before the other would cause it to fail. Swapping the parts of the test name around I think they still read well and avoid that problem. How insiduous that kind of failure could be. Add it to the least of things I didn’t expect to see today.
michaeldjeffrey
force-pushed
the
mj/fileupload-test-race
branch
from
dfb8c92 to
5f4f7c4
Compare
Merged
bbalser
reviewed
Apr 24, 2026
bbalser
approved these changes
Apr 24, 2026
There’s a chance for some of these test that we trigger a shutdown of the daemon after a file has been processed, but before FileUpload was able to fully upload it to s3. Given our pattern is to look for orphaned files and try to upload them on startup, I didn’t want to change the contract that much for currently running services by making FileUpload wait until it’s upload channel was complete before shutting down. So we’ve added a small counter where you can ask it how many files have been uploaded. In a test situation that should be a predictable thing, if there’s a case where it’s not, I can't predict that right now and whoever runs into that will need to come up with a better solution. I had a thought that mybe the watch channel would hold a map of file_types to the number uploaded, so you could get a more granular. But that’s not needed for me right now. Currently playing: Angine De Poitine Vol.II I haven’t spent much time listening to microtonal music. But it’s giving me a weird sense of comfort right now. There are so many unsurities flying around right now. Maybe that’s why? Who’s to know.
michaeldjeffrey
force-pushed
the
mj/fileupload-test-race
branch
from
5f4f7c4 to
232eac1
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
There’s a chance for some of these test that we trigger a shutdown of
the daemon after a file has been processed, but before FileUpload was
able to fully upload it to s3.
Given our pattern is to look for orphaned files and try to upload them
on startup, I didn’t want to change the contract that much for currently
running services by making FileUpload wait until it’s upload channel was
complete before shutting down. So we’ve added a small counter where you
can ask it how many files have been uploaded. In a test situation that
should be a predictable thing, if there’s a case where it’s not, I can't
predict that right now and whoever runs into that will need to come up
with a better solution.