Add class to mock the AWS JS SDK for unit testing #17
Conversation
| @@ -51,6 +51,7 @@ module.exports = function(options, onSuccess, onError) { | |||
| // If there's still an error, it's not a file or a directory, so call | |||
| // the error callback. | |||
| if (error) { | |||
| console.log(error); | |||
| S3.prototype.listObjects = function(parameters, callback) { | ||
| var path = '/' + parameters.Prefix; | ||
|
|
||
| wdfs.readDirectory({ |
There was a problem hiding this comment.
This looks behaving differently.
In my understanding, listObjects will return the list of the all objects with the prefix, so if s3 storage has the following files:
foo/bar.txt
foo/baz/quox.txt
foo/baz/foobar.txt
baz/
and when requested with prefix "foo/", the result is foo/bar.txt, foo/baz/quox.txt, foo/baz/foobar.txt. And your s3 client parses the result as foo/ has "bar.txt" and a directory "baz".
However, this mock doesn't dig into the directory, so response would be "foo/bar.txt" and "foo/baz/" directory.
This difference would skew the test result.
There was a problem hiding this comment.
No, I think listObjects returns only files that are immediate children of the prefix/directory. Any sub prefixes/directories are returned in the CommonPrefixes list, which you can see below.
There was a problem hiding this comment.
by the way, then better to check if parameters contain 'Delimiter' field?
There was a problem hiding this comment.
The Delimiter field is only used internally by the actual S3 server. Since we're using WebDAV as the backend, and the only delimiter supported is '/', we can emulate the behaviour without checking the field itself.
I don't think this is a good place to do error checking, such as for missing parameters, since it's only used by the test suite - if a parameter isn't supplied by a call made in the test suite then that's a problem with the test, not the main code.
There was a problem hiding this comment.
S3 server groups the result only when Delimiter is set, and your mock only emulates this behavior.
So I am suggesting to check if Delimiter is set to '/' and fails otherwise. That would not break the test at all because all of your code is currently setting Delimiter correctly, but it seems worth checking so.
There was a problem hiding this comment.
So what, just console.error a message and return?
I can't pass an error to the callback because that's not consistent with the S3 API I'm mocking
There was a problem hiding this comment.
I don't think console.error is great for tests. What about invoking callback with meaningful error message and no data?
Then the test will fail in that case.
There was a problem hiding this comment.
Okay. It's not consistent with what the S3 library would do but it works for this case.
|
lgtm |
Add class to mock the AWS JS SDK for unit testing
This PR deals with #5.
Not finished yet, just starting a PR to kick off discussion.
Todo:
getObjectresponse body class with itstoArrayBuffermethod.