-
Notifications
You must be signed in to change notification settings - Fork 761
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
Comments
I figured it out. I just added the 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();
});
}); |
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..
All my other tests run fine. I have two other tests that implement 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();
});
});
}); |
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.
I had the same exact issue, but with a different root cause I believe. |
See ladjs/supertest/issues/230#issuecomment-103226606
I have this same issue, but neither solution works for me :( |
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.
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
|
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.
I test this API:
With this test:
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:
The API works as expected while uploading a tiff image in Chrome:
Details:
I've never seen anything like that before. The random behavior with the same code is really weird.
The text was updated successfully, but these errors were encountered: