dwyl / aws-sdk-mock

:rainbow: AWSomocks for Javascript/Node.js aws-sdk tested, documented & maintained. Contributions welcome!
Apache License 2.0
1.11k stars 110 forks source link

S3 getObject Mocking failing (2 cases, 1 work around) #84

Open dugooder opened 7 years ago

dugooder commented 7 years ago

The first test isn't a big deal, the work around is easy - keep to a standard way to call getObject. The second (streaming a object) is a show stopper so any advice would be appreciated. Hopefully, I am doing sometihng wrong with the mock. Thanks for making / maintaining this module, it has been a great help.

`"use strict"; / package.json { "name": "aws-issues", "version": "1.0.0", "description": "", "main": "app.js", "scripts": { "test": "test.js" }, "author": "dugooder", "license": "ISC", "dependencies": { "aws-sdk": "^2.41.0", "aws-sdk-mock": "^1.6.1", "chai": "^3.5.0", "mocha": "^3.2.0" } } /

const expect = require("chai").expect; const fs = require("fs");

describe("while mocking aws getObject", function () { it.only("passing the parameters in the S3 constructors fails", function (done) { const AWS = require("aws-sdk"); const awsMock = require("aws-sdk-mock");

    // Error thrown if passing param via S3 object constructor.
    //  The mocking frameworks expect the params to be passed via 
    //  getObject(params,function) but they can be passed
    //  via the AWS.S3 constructor.
    // I believe the code should look to both. 
    //
    // new _AWS.ParamValidator((client.config ||
    //    _AWS.config).paramValidation).validate(inputRules, params || client.config );
    //
    // Until the code is changed setting the paramValdiation 
    //  to false will skip the issue and allow mocking to proceed.
    //
    //    AWS.config.paramValidation = false;
    // 
    // My resolution: Changed my code to pass the params in the getobject call.

    awsMock.mock("S3", "getObject", { name: "first" });

    var s3Params = { Bucket: "bucketName", "Key": "objectkey" };
    var s3 = new AWS.S3(s3Params);
    //WORK AROUND: var s3 = new AWS.S3();

    // on s3.getObject an error is thrown but it does not bubble up.
    // * MissingRequiredParameter: Missing required key 'Bucket' in params
            // * MissingRequiredParameter: Missing required key 'Key' in params
    s3.getObject(function (err, data) {
        //WORK AROUND: s3.getObject(s3Params, function (err, data) {
        //FAILS: Cannot read property 'name' of null
        expect(data.name).equal("first");
        done();
    });
});

it("using streaming deos not deliver the mocked data", function (done) {
    const AWS = require("aws-sdk");
    const awsMock = require("aws-sdk-mock");

    awsMock.mock("S3", "getObject", function (param, cb) {
        // TRY 1: var data = {"Body" : new Buffer("file contents here")};
        // TRY 2: var data = new Buffer("file contents here");
        // TRY 3: 
        var Readable = require("stream").Readable;
        var data = new Readable;
        data.push("file contents here");
        data.push(null);
        cb(null, data);
        // TRY 3 END:
    });

    //TRY 4
    // var Readable = require("stream").Readable;
    // var data = new Readable;
    // data.push("file contents here");
    // data.push(null);
    // awsMock.mock("S3", "getObject", data);
    //TRY 4 END

    var s3 = new AWS.S3();

    var params = { Bucket: "myBucket", Key: "myFile.txt" };
    var filePathName = __dirname + "/fle.txt";
    var file = fs.createWriteStream(filePathName);
    // Why do we do it his way?  Security requirements are high. 
    //  Our S3 bucket is locked down so direct access is not acceptable.
    //  Streaming directly to file instead of getting the data
    //  via the function (err, data) and writing it to a file 
    //  would appears to be a better approach.   
    //  Memory and reliable with larger files are better 
    //  with the streamming approach (from our experience).
    s3.getObject(params).
        createReadStream().
        on("error", function (err) {
            expect(err).to.not.exist;
            done();
        }).
        on("end", function () {
            var fscontents = fs.readFileSync(filePathName, "utf8");
            // FAILS, fscontents == ""
            expect(fscontents).equal("file contents here");  
            done();
        }).
        pipe(file);
});

});`

nagapavan commented 6 years ago

+1 Having the similar problem in testing S3 getObject as stream.

wdinau commented 6 years ago

same here:

awsMock.mock("S3", "getObject", (params, callback) => {
        callback(null, require("fs").createReadStream("testFile.csv"));
});

0 bytes return.

nagapavan commented 6 years ago

I got the issue fixed. From the aws-sdk-mock code, we have to return a Buffer instead of readStream to get the mock done. Please try following:

  awsMock.mock("S3", "getObject", new Buffer(require("fs").readFileSync("testFile.csv")));
wdinau commented 6 years ago
awsMock.mock("S3", "getObject", new Buffer(require("fs").readFileSync("testFile.csv")));

^ The above definitely works. Thank you!

In the previous question, I did not make use of the param in order to simplify the problem. In reality, I need the return file name to be defined by the parameter i.e.

awsMock.mock("S3", "getObject",(params, callback) =>{
  callback(null, new Buffer(require("fs").readFileSync(params.fileName)))
});

I tried several varients without success. Am I using this right?

nagapavan commented 6 years ago

I am not sure if callback works in this case. Can you try following?

awsMock.mock("S3", "getObject",(params, callback) =>{
  callback(new Buffer(require("fs").readFileSync(params.fileName)))
});
nagapavan commented 6 years ago

My apologies, Code wouldn't allow that to work:

createReadStream() {
    const stream = new Readable();
    stream._read = function (size) {
        if (typeof (replace) === 'string' || Buffer.isBuffer(replace)) {
            this.push(replace);
        }
        this.push(null);
    };
    return stream;
}

Please try using separate mocks to read individual files, in respective test case - if that works for you. It would bloat up the file, but might work.

wdinau commented 6 years ago

Sorry, I didn't quite follow the above suggestion on a "createReadStream" mock. It probably will make my test case hard to read for team members.

It boils down to 2 choices:

  1. Continue to use aws-sdk-mock, mock

    AWS.S3().getObject(params)

    <-- return new Buffer works, but couldn't get 'params' passed in and used. Is this something quite tricky to fix?

  2. Give up mocking aws, extract the hard-to-mock-bit into a method and mock it to return a file base on param.

    
    // code.js
    getAWSObject(fileName: string) {
    const params = {
      Bucket: env.BUCKET_NAME,
      Key: fileName
    };
    
    return new AWS.S3().getObject(params).createReadStream();
    }

// test.js: getAWSObject = jest.fn(fileName => { return fs.createReadStream(testFilePath + fileName); });


I'm taking choice 2 at the moment. Would like to learn what challenges are there to return a callback with a Readable stream.

Thanks.
nckswt commented 4 years ago

I got the issue fixed. From the aws-sdk-mock code, we have to return a Buffer instead of readStream to get the mock done. Please try following:

  awsMock.mock("S3", "getObject", new Buffer(require("fs").readFileSync("testFile.csv")));

I spent about 4 hours trying to figure out how to properly set up the createReadStream mock before finding this. This finally worked! Thanks for posting it! Could definitely use more visibility, though.

Two things I haven't been able to do, though, are pass a sinon stub and passing a stream.

Passing a stream

I want to test stream errors, and I was hoping something like this might work, but that doesn't seem to be the case:

const readStream = fs.createReadStream(FILE_KEY);
readStream.on('data', () => readStream.emit('error', new Error('Something bad happened')));
aws.mock('S3', 'getObject', readStream);

Passing a Sinon stub

I want to validate the arguments of getObject, so I tried a few different variations of

const stub1 = sandbox.stub().returns(Buffer.from(fs.readFileSync(FILE_KEY)));
const stub2 = sandbox.stub().returns(((_, cb) => Buffer.from(fs.readFileSync(FILE_KEY))));
const stub3 = sandbox.stub().returns(((_, cb) => cb(null, Buffer.from(fs.readFileSync(FILE_KEY)))));
const stub = stub1;
aws.mock('S3', 'getObject', stub);

... 

expect(stub).to.have.been.calledOnceWith({
  Bucket: 'some-bucket',
  Key: FILE_KEY,
});

and I never seemed to successfully stub the method. Am I missing something?

jiayiyang1997 commented 2 years ago

I got the issue fixed. From the aws-sdk-mock code, we have to return a Buffer instead of readStream to get the mock done. Please try following:

  awsMock.mock("S3", "getObject", new Buffer(require("fs").readFileSync("testFile.csv")));

I spent about 4 hours trying to figure out how to properly set up the createReadStream mock before finding this. This finally worked! Thanks for posting it! Could definitely use more visibility, though.

Two things I haven't been able to do, though, are pass a sinon stub and passing a stream.

Passing a stream

I want to test stream errors, and I was hoping something like this might work, but that doesn't seem to be the case:

const readStream = fs.createReadStream(FILE_KEY);
readStream.on('data', () => readStream.emit('error', new Error('Something bad happened')));
aws.mock('S3', 'getObject', readStream);

Passing a Sinon stub

I want to validate the arguments of getObject, so I tried a few different variations of

const stub1 = sandbox.stub().returns(Buffer.from(fs.readFileSync(FILE_KEY)));
const stub2 = sandbox.stub().returns(((_, cb) => Buffer.from(fs.readFileSync(FILE_KEY))));
const stub3 = sandbox.stub().returns(((_, cb) => cb(null, Buffer.from(fs.readFileSync(FILE_KEY)))));
const stub = stub1;
aws.mock('S3', 'getObject', stub);

... 

expect(stub).to.have.been.calledOnceWith({
  Bucket: 'some-bucket',
  Key: FILE_KEY,
});

and I never seemed to successfully stub the method. Am I missing something?

Hi Nick, thank you for your comment! Could you please share how you manage to test stream errors? I was stuck in the same problem too. Tried to use stream but it doesn't work.