Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

Socket.io Testing #865

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions config/lib/socket.io.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ module.exports = function (app, db) {
// Get the session id from the request cookies
var sessionId = socket.request.signedCookies ? socket.request.signedCookies[config.sessionKey] : undefined;

console.log('sessionId from socket.io.js:76 -->',sessionId);

if (!sessionId) return next(new Error('sessionId was not found in socket.request'), false);

// Use the mongoStorage instance to get the Express session information
Expand Down
115 changes: 113 additions & 2 deletions modules/chat/tests/server/chat.socket.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,117 @@
/**
* Chat socket tests
*/
describe('Chat Socket Tests:', function () {
// TODO: Add chat socket tests

var should = require('should'),
request = require('supertest'),
path = require('path'),
mongoose = require('mongoose'),
sinon = require('sinon'),
User = mongoose.model('User'),
express = require(path.resolve('./config/lib/express')),
io = require('socket.io/node_modules/socket.io-client');

/**
* Globals
*/
var app, agent, credentials, ioc, user, admin, sessionId, clock;

describe('Socket Chat tests:', function() {

// Get application
before(function(done) {
app = express.init(mongoose);
agent = request.agent(app);

done();
});

// create the user
beforeEach(function(done) {
// Create user credentials
credentials = {
username: 'username',
password: 'password'
};

// Create a new user
user = new User({
firstName: 'Full',
lastName: 'Name',
displayName: 'Full Name',
email: '[email protected]',
username: credentials.username,
password: credentials.password,
provider: 'local'
});

// Save a user to the test db
user.save(function () {
done();
});
});

it('should be able to connect on socket', function(done) {
clock = sinon.useFakeTimers(Date.now());
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added sinon.useFakeTimers as we should use it anyway when testing. In 1 of 10 cases my tests fail because of some latency. Maybe we can shift to this when testing objects where time is just on of the attributes.


var testMessage = {
created: Date.now(),
profileImageURL: 'modules/users/client/img/profile/default.png',
text: 'Is now connected',
type: 'status',
username: 'username'
};

agent.post('/api/auth/signin')
.send(credentials)
.expect(200)
.end(function (signinErr, res) {
// console.log('---agent: ',agent);

// Handle signin error
if (signinErr) {
return done(signinErr);
}

var socket, cookies;
if (res.headers && res.headers['set-cookie'] && res.headers['set-cookie'].length > 0) {
cookies = res.headers['set-cookie'].join(';');
}

if (cookies) {
socket = io('http://localhost:3001', {
extraHeaders: {
'Cookie': cookies
}
});
} else {
socket = io('http://localhost:3001');
}

// socket.on('disconnect', function () {
// console.log('disconnected');
// });

socket.on('error', function (err) {
return done(err);
});

socket.on('connect', function () {
// console.log('connected');

socket.on('chatMessage', function (data) {
console.log('test-result from chatMessage (chat.socket.tests.js:105) -->',data);
data.should.eql(testMessage);
done();
});
});

});

});

afterEach(function(done) {
User.remove().exec(done);
clock.restore();
});
});
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
"passport-twitter": "^1.0.2",
"phantomjs": ">=1.9.0",
"serve-favicon": "^2.3.0",
"socket.io": "^1.3.5",
"socket.io": "Automattic/socket.io",
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little bit concerned and maybe we should wait for the next socket.io version. Unclear when it's released tho.

This line equals "socket.io": "socketio/socket.io",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastianwegge I'm skeptical about having the development version of socket.io brought into the project.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codydaig I was talking to @mleanos about that. What we need would be the development version of socket.io-client for testing, since it has the extraHeaders included. This is the only reasonable way to test socket.io at the moment. Other ways include varieties of hacks, that are not worth including. I'll give this a shot in about 4-5 hours (when it's evening time and I'm off work).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastianwegge I agree that's the most reasonable way of testing. Using extraHeaders is what @mleanos and I came up with while debugging, it just isn't in the stable version yet.

IMO, if you want to write the tests for socket.io and put them in the repository as pending, that's fine. But I don't want the project to rely on a development version of a dependency.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @codydaig that we don't want to introduce the development version of socket.io. I think there's a simple solution to this issue. How about we just install the development version of socket.io-client? Socket.IO exposes the internal socket.io-client that's installed with the package which is exposed to this app via https://github.com/meanjs/mean/blob/master/modules/core/server/views/layout.server.view.html#L52

If we install the stand-alone socket.io-client then we can use this version in our tests. I believe our app will still use the socket.io-client installed with socket.io. Although, I'm not sure of this; my initial guess is that it won't have any impact at all. However, maybe someone else has some insight into that concern.

So the changes to package.json would look like this

  "socket.io": "1.3.6",
  "socket.io-client":  "https://github.com/socketio/socket.io-client#master"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to use socket.io-client head for testing until extraHeaders is on release

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@codydaig pointed out that the socket.io-client package isn't installed with socket.io if the stand-alone socket.io-client is installed. So given that behavior, my above proposal may not be viable.

Perhaps, we will just have to wait for engine.io-client to update the npm package to their latest version. I wouldn't expect to be waiting too long for this to happen.

In the meantime, we could work on building these tests & polishing up this PR in anticipation for the engine.io-client project to be updated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't you checkout the difference from the 1.3.6 to the head was the dependency of the socket.io-client?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bastianwegge Yes. I saw that. I was hoping that we could install the stand-alone HEAD version of socket.io-client, and still use the version installed with socket.io. Guess that's not the case, as Cody pointed out. I say we sit tight and work on these tests in the meantime.

"swig": "^1.4.2",
"validator": "^3.41.2"
},
Expand Down Expand Up @@ -107,6 +107,7 @@
"load-grunt-tasks": "^3.2.0",
"run-sequence": "^1.1.1",
"should": "^7.0.1",
"sinon": "^1.16.1",
"supertest": "^1.0.1"
}
}