Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random test success/failure on Windows #230

Closed
Shingaz opened this issue May 18, 2015 · 5 comments
Closed

Random test success/failure on Windows #230

Shingaz opened this issue May 18, 2015 · 5 comments

Comments

@Shingaz
Copy link

Shingaz commented May 18, 2015

I test this API:

function handleError(res){
    return function(data) {
        res.status(data.status).json(data.error);
    };
}

function reject(deferred, error, statusCode) {
    deferred.reject({
        status: statusCode || error.status || 500,
        error: error.payload
    });
}

function responseWithResult(res){
    return function(data) {
        if (data.entity) {
            return res.status(data.statusCode).json(data.entity);
        }
        res.status(500).json(error.UNKNOWN_ERROR.payload);
    };
}

function resolve(deferred, entity, statusCode) {
    deferred.resolve({
        statusCode: statusCode || 200,
        entity: entity
    });
}

function validate(deferred, filename, mimetype) {
    if (/.(?:jpe?g|png|gif)$/.test(filename.toLowerCase()) === false) {
    // Should reply here with status code 400 and body: { "code": 90, "message": "The file extension is not supported. Accepted extensions: jpg, png and gif." }
        reject(deferred, error.FILE_EXTENSION_NOT_SUPPORTED, 400); 
        return false;
    }
    if (/^image\/.+$/.test(mimetype) === false) {
        reject(deferred, error.WRONG_MIME_TYPE, 400);
        return false;
    }
    return true;
}

exports.create = function(req, res) {
    var deferredResponse = Q.defer();
    var image;

    deferredResponse.promise
        .then(responseWithResult(res))
        .catch(handleError(res));

    var form = new busboy({
        headers: req.headers,
        limits: {
            fields: 1,
            parts: 2,
            files: 1,
            fileSize: 5 * 1024 * 1024
        }
    })
    .on('file', function(fieldname, file, filename, encoding, mimetype) {
        if (validate(deferredResponse, filename, mimetype)) {
            image = uniqueFilename(path.extname(filename));
            file.pipe(fs.createWriteStream(path.join(IMAGES_FOLDER, image)));
        }
        file.on('data', function(data) {});
        file.on('end', function() {
            if (file.truncated) {
                reject(deferredResponse, error.FILE_SIZE_LIMIT_EXCEEDED);
            }
        });
    })
    .on('error', function() {
        reject(deferredResponse, error.BAD_PARSING);
    })
    .on('finish', function() {
        resolve(deferredResponse, { filename: image }, 201);
    });
    req.pipe(form);
};

With this test:

it('should respond with with 400 and the error code 90 on unsupported file extension', function(done) {
    request(app)
        .post('/api/images')
        .set('authorization', 'Bearer ' + token)
        .attach('file', path.join(TEST_FOLDER, 'test.tiff'))
        .expect(400)
        .expect('Content-Type', /json/)
        .end(function(err, res) {
            if (err) return done(err);
            expect(res.body.code).to.equal(90);
            done();
        });
});

It always success on MacOS and TravisCI but when I run it on Windows, it fails most of time (6/10 times) and returns the error:

  1) Image API: POST /api/images should respond with with 400 and the error code 90 on unsupported file extension:
     Uncaught TypeError: Cannot read property 'header' of undefined
      at Test.assert (<project_root>\node_modules\supertest\lib\test.js:188:17)
      at Server.assert (<projet_root>\node_modules\supertest\lib\test.js:131:12)
      at Server.g (events.js:199:16)
      at Server.emit (events.js:104:17)
      at net.js:1409:10
      at process._tickDomainCallback (node.js:381:11)

The API works as expected while uploading a tiff image in Chrome:
image

Details:

  • Node v0.12.2
  • Windows 8.1 x64
  • grunt-mocha-test 0.12.7
  • supertest 1.0.1
  • chai-as-promised 5.0.0
  • chai-things 0.2.0
  • sinon-chai 2.7.0

I've never seen anything like that before. The random behavior with the same code is really weird.

@Shingaz
Copy link
Author

Shingaz commented May 18, 2015

I figured it out. I just added the "Connection: keep-alive" header in the request. If someone has an explanation, I would be happy to know it!

it('should respond with with 400 and the error code 90 on unsupported file extension', function(done) {
    request(app)
        .post('/api/images')
        .set('authorization', 'Bearer ' + token)
        .set('Connection', 'keep-alive')
        .attach('file', path.join(TEST_FOLDER, 'test.tiff'))
        .expect(400)
        .expect('Content-Type', /json/)
        .end(function(err, res) {
            if (err) return done(err);
            expect(res.body.code).to.equal(90);
            done();
        });
});

@Shingaz Shingaz closed this as completed May 18, 2015
@mleanos
Copy link

mleanos commented Oct 28, 2015

I'm having a similar issue, and the suggested fix won't work for me. If I keep the connection open, my test will timeout. I've verified that this is only an issue on Windows.

API method:

exports.changeProfilePicture = function (req, res) {
  var user = req.user;
  var message = null;
  var upload = multer(config.uploads.profileUpload).single('newProfilePicture');
  var profileUploadFileFilter = require(path.resolve('./config/lib/multer')).profileUploadFileFilter;

  // Filtering to upload only images
  upload.fileFilter = profileUploadFileFilter;

  if (user) {
    upload(req, res, function (uploadError) {
      if(uploadError) {
        return res.status(400).send({
          message: 'Error occurred while uploading profile picture'
        });
      } else {
        user.profileImageURL = config.uploads.profileUpload.dest + req.file.filename;

        user.save(function (saveError) {
          if (saveError) {
            return res.status(400).send({
              message: errorHandler.getErrorMessage(saveError)
            });
          } else {
            req.login(user, function (err) {
              if (err) {
                res.status(400).send(err);
              } else {
                res.json(user);
              }
            });
          }
        });
      }
    });
  } else {
    res.status(400).send({ // I have verified that the test makes it here every time
      message: 'User is not signed in'
    });
  }
};

Test:

it('should not be able to change profile picture if not signed in', function (done) {
    agent.post('/api/users/picture')
      .attach('newProfilePicture', './modules/users/client/img/profile/default.png')
      .send(credentials)
      .expect(400)
      .end(function (userInfoErr, userInfoRes) {
        done(userInfoErr);
      });
  });

This is the error I receive, about 60% of the time when running this test..

 1) User CRUD tests should not be able to change profile picture if not signed in:
    TypeError: Cannot read property 'status' of undefined
     at Test._assertStatus (project\node_modules\supertest\lib\test.js:229:10)
     at Test._assertFunction (project\node_modules\supertest\lib\test.js:247:11)
     at Test.assert (project\node_modules\supertest\lib\test.js:148:18)
     at Server.assert (project\node_modules\supertest\lib\test.js:127:12)
     at Server.g (events.js:199:16)
     at Server.emit (events.js:129:20)
     at net.js:1419:10
     at process._tickDomainCallback (node.js:381:11)

All my other tests run fine. I have two other tests that implement .attach on the request, but this is the only one that has an issue.

The other tests that never produce this error:

it('should not be able to change profile picture if attach a picture with a different field name', function (done) {
    agent.post('/api/auth/signin')
      .send(credentials)
      .expect(200)
      .end(function (signinErr, signinRes) {
        // Handle signin error
        if (signinErr) {
          return done(signinErr);
        }

        agent.post('/api/users/picture')
          .attach('fieldThatDoesntWork', './modules/users/client/img/profile/default.png')
          .send(credentials)
          .expect(400)
          .end(function (userInfoErr, userInfoRes) {
            done(userInfoErr);
          });
      });
  });

it('should be able to change profile picture if signed in', function (done) {
    agent.post('/api/auth/signin')
      .send(credentials)
      .expect(200)
      .end(function (signinErr, signinRes) {
        // Handle signin error
        if (signinErr) {
          return done(signinErr);
        }

        agent.post('/api/users/picture')
          .attach('newProfilePicture', './modules/users/client/img/profile/default.png')
          .send(credentials)
          .expect(200)
          .end(function (userInfoErr, userInfoRes) {
            // Handle change profile picture error
            if (userInfoErr) {
              return done(userInfoErr);
            }

            userInfoRes.body.should.be.instanceof(Object);
            userInfoRes.body.profileImageURL.should.be.a.String();
            userInfoRes.body._id.should.be.equal(String(user._id));

            return done();
          });
      });
  });

mleanos added a commit to mleanos/mean that referenced this issue Oct 29, 2015
Removes a duplicate User CRUD test for Profile Picture.

There are two reasons for this commit.

1) Duplicate of
https://github.com/meanjs/mean/blob/master/modules/users/tests/server/user.server.routes.tests.js#L833-L848
2) This test is problematic in Windows environment.
Related to:
ladjs/supertest#230
ladjs/supertest#258

The latter may be an issue with the `.attach` method not completely
loading the file into memory before the 400 status response is sent back
due to no User logged in.
@Overdrivr
Copy link

I had the same exact issue, but with a different root cause I believe.
I was using malformed paths like for instance api/auth/signin/ instead of /api/auth/signin.
As soon as I switched to the second format tests ran fine.
Hope this helps.

davidcheung pushed a commit to strongloop/loopback-component-storage that referenced this issue Aug 3, 2016
davidcheung pushed a commit to strongloop/loopback-component-storage that referenced this issue Aug 3, 2016
davidcheung pushed a commit to strongloop/loopback-component-storage that referenced this issue Aug 3, 2016
@timendez
Copy link

timendez commented Oct 7, 2017

I have this same issue, but neither solution works for me :(

kgrossnickle pushed a commit to cen3031-5a/trainerScheduling that referenced this issue Oct 10, 2017
Removes a duplicate User CRUD test for Profile Picture.

There are two reasons for this commit.

1) Duplicate of
https://github.com/meanjs/mean/blob/master/modules/users/tests/server/user.server.routes.tests.js#L833-L848
2) This test is problematic in Windows environment.
Related to:
ladjs/supertest#230
ladjs/supertest#258

The latter may be an issue with the `.attach` method not completely
loading the file into memory before the 400 status response is sent back
due to no User logged in.
@fanjieqi
Copy link

fanjieqi commented Apr 11, 2018

Same with @timendez, neither solution works for me. I have no idea why the crud test Random success/failure on docker Jenkins. The post and put request often fail. The error log is

08:18:25   2) EmployeeScore API
08:18:25        GET /employee_scores/:id
08:18:25          should return employeeScore:
08:18:25      TypeError: Cannot read property 'header' of undefined
08:18:25       at Test._assertHeader (node_modules/supertest/lib/test.js:232:20)
08:18:25       at Test._assertFunction (node_modules/supertest/lib/test.js:281:11)
08:18:25       at Test.assert (node_modules/supertest/lib/test.js:171:18)
08:18:25       at assert (node_modules/supertest/lib/test.js:131:12)
08:18:25       at /var/jenkins_home/workspace/employee_scores/employee_scores/node_modules/supertest/lib/test.js:128:5
08:18:25       at Test.Request.callback (node_modules/superagent/lib/node/index.js:718:3)
08:18:25       at ClientRequest.req.once.err (node_modules/superagent/lib/node/index.js:646:10)
08:18:25       at Socket.socketErrorListener (_http_client.js:395:9)
08:18:25       at emitErrorNT (internal/streams/destroy.js:64:8)
08:18:25       at process._tickCallback (internal/process/next_tick.js:178:19)
08:18:25 
08:18:25   3) EmployeeScore API
08:18:25        DELETE /employee_scores/:id
08:18:25          should return employeeScore:
08:18:25      TypeError: Cannot read property 'header' of undefined
08:18:25       at Test._assertHeader (node_modules/supertest/lib/test.js:232:20)
08:18:25       at Test._assertFunction (node_modules/supertest/lib/test.js:281:11)
08:18:25       at Test.assert (node_modules/supertest/lib/test.js:171:18)
08:18:25       at assert (node_modules/supertest/lib/test.js:131:12)
08:18:25       at /var/jenkins_home/workspace/employee_scores/employee_scores/node_modules/supertest/lib/test.js:128:5
08:18:25       at Test.Request.callback (node_modules/superagent/lib/node/index.js:718:3)
08:18:25       at ClientRequest.req.once.err (node_modules/superagent/lib/node/index.js:646:10)
08:18:25       at Socket.socketErrorListener (_http_client.js:395:9)
08:18:25       at emitErrorNT (internal/streams/destroy.js:64:8)
08:18:25       at process._tickCallback (internal/process/next_tick.js:178:19)

lupinthethirdgentleman pushed a commit to lupinthethirdgentleman/mean-dashboard that referenced this issue Aug 5, 2021
Removes a duplicate User CRUD test for Profile Picture.

There are two reasons for this commit.

1) Duplicate of
https://github.com/meanjs/mean/blob/master/modules/users/tests/server/user.server.routes.tests.js#L833-L848
2) This test is problematic in Windows environment.
Related to:
ladjs/supertest#230
ladjs/supertest#258

The latter may be an issue with the `.attach` method not completely
loading the file into memory before the 400 status response is sent back
due to no User logged in.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants