alexmojaki / s3-stream-upload

Manages streaming of data to AWS S3 without knowing the size beforehand and without keeping it all in memory or writing to disk.
MIT License
208 stars 62 forks source link

Update AWS SDK to V2 #43

Open alexwhiteoval opened 2 years ago

alexwhiteoval commented 2 years ago
alexmojaki commented 2 years ago

It looks like integrity checking is no longer being tested, and that mocking makes it difficult to do so. That seems like a significant problem.

alexwhiteoval commented 2 years ago

OK, I can have a look at this but it won't be today.

alexwhiteoval commented 2 years ago

I have updated the tests to include integrity checking

alexmojaki commented 2 years ago

The test doesn't actually check the uploaded content. The list of StringBuilders is never examined. It looks like mocking has been taken too far. If setting up some local equivalent to S3 like s3proxy or localstack is too much trouble, then I'd prefer a test against the real S3.

alexwhiteoval commented 2 years ago

That is correct I have written it as a unit test and didn't intend on testing the AWS SDK, only the code in the function. The mocking allows for testing that each part upload matches and that the upload ordering returns the correct hash at the end of the upload. The hashes are coming from the uploaded content as they do from AWS.

If you would like an integration test that includes the AWS SDK then that is fine, do you have a preference for using an s3proxy or localstack?

alexmojaki commented 2 years ago

I don't have any preference, I imagine there's many more similar S3 'emulators' out there that could work. The original s3proxy test was full of boilerplate that I didn't really understand, which I'd rather avoid, but using s3proxy nowadays may or may not require that.