Tests fail on Windows

[addisonElliott]

Issue Description

Under Windows, when running the test suite for parse-server, many of the tests will fail with the following error message:

Failed: There were open connections to the server left after the test finished

However, running the exact same test suite under Linux (in my case, I tried it on Debian amd64), the tests pass fine.

When running all of the tests, about 60% of them will fail with this error message. I'll explain my theory why below but this is not a small issue by any means.

Steps to reproduce

1. Clone parse-server repository.
2. Run npm install in the directory.
3. Run npm run test:win for Windows and the errors should appear.
4. If you are testing on Linux, you do npm run test or npm test/

Expected Results

This error message occurs with many files so I just picked one test file where this occurred and debugged that. The first test file ran is AccountLockoutPolicy.spec.js and the very first test account should not be locked even after failed login attempts if account lockout policy is not set fails on Windows.

For either Linux or Windows, the expectation is for the tests to pass. Here is the output for Linux when testing just the AccountLockoutPolicy.spec.js test file. Note that I stop it halfway through because I am focusing on that first test.

Jasmine started
1
2
3
Open connection127.0.0.1:45278
Close connection127.0.0.1:45278
4
Open connection127.0.0.1:45280
Close connection127.0.0.1:45280
5
Open connection127.0.0.1:45284
Close connection127.0.0.1:45284
6
Open connection127.0.0.1:45286
Close connection127.0.0.1:45286
6.1
6.2
Open connection127.0.0.1:45288
Close connection127.0.0.1:45288

  Account Lockout Policy:
    ✓ account should not be locked even after failed login attempts if account lockout policy is not set
    ✓ throw error if duration is set to an invalid number
    ✓ throw error if threshold is set to an invalid number
    ✓ throw error if threshold is < 1
    ✓ throw error if threshold is > 999
    ✓ throw error if duration is <= 0
    ✓ throw error if duration is > 99999
Open connection127.0.0.1:45290
Close connection127.0.0.1:45290
Open connection127.0.0.1:45296
Close connection127.0.0.1:45296
Open connection127.0.0.1:45298
Close connection127.0.0.1:45298
Open connection127.0.0.1:45300
Close connection127.0.0.1:45300
Open connection127.0.0.1:45302
Close connection127.0.0.1:45302
    ✓ lock account if failed login attempts are above threshold
Open connection127.0.0.1:45304
Close connection127.0.0.1:45304
Open connection127.0.0.1:45306
^C

On Windows, here is the result running the tests on the same file:

Jasmine started
1
2
3
Open connection127.0.0.1:5331
4
Close connection127.0.0.1:5331
Open connection127.0.0.1:5332
5
Close connection127.0.0.1:5332
Open connection127.0.0.1:5333
6
Close connection127.0.0.1:5333
Open connection127.0.0.1:5334
6.1
6.2
Close connection127.0.0.1:5334
Open connection127.0.0.1:5335

  Account Lockout Policy:
    × account should not be locked even after failed login attempts if account lockout policy is not set
      - Failed: There were open connections to the server left after the test finished
Close connection127.0.0.1:5335
Terminate batch job (Y/N)? Y

What I think the issue is

When digging into the code to diagnose this issue, I started with the error message and what it indicates. In spec/helper.js, lines 198-201 are the following:

  const afterLogOut = () => {
    console.log("afterLogOut: 1");
    if (Object.keys(openConnections).length > 0) {
      fail('There were open connections to the server left after the test finished');
    }

So there is an array of open connections and if that is not empty at the end of the test, then something is still running or having issues. So, the next step was I found where openConnections was changed. Lines 143-150 in the same helper file are:

      server = parseServer.server;
      server.on('connection', connection => {
        const key = `${connection.remoteAddress}:${connection.remotePort}`;
        openConnections[key] = connection;
        console.log("Open connection" + key);
        console.trace(); 
        connection.on('close', () => { console.log("Close connection" + key); console.trace(); delete openConnections[key]; });
      });

If you take a look at the logs given above, for Linux the connection is opened and closed almost immediately which is what you expect for HTTP requests. However, Windows has a bit of a lag and this is what causes the error. When the open connections are checked, there is still an open connection because it has not been closed yet.

Sample test case

I was not sure if the error was related to Parse or a bug in XMLHttpRequest so I created a test script to determine if the same thing occurs with XMLHttpRequest.

Here is my test script where the same issue still occurs:

var XMLHttpRequest = require('xmlhttprequest').XMLHttpRequest,
	express = require('express'),
	http = require('http'),
	Url = require("url");

function makeRequest (method, url) {
	return new Promise(function (resolve, reject) {
		var xhr = new XMLHttpRequest();
		var handled = false;

		xhr.onreadystatechange = function() {
			if (xhr.readyState !== 4 || handled) {
				return;
			}

			handled = true;
			console.log("ajax: 1", xhr.readyState);
			resolve(xhr.responseText, xhr.status, xhr);
			console.log("ajax: 2", xhr.readyState);
		};

		xhr.open(method, url, true);
		xhr.send();
	});
}

var app = express();
var sockets = {};
var server = app.listen(1337, 'localhost', () => console.log('Example app listening on port 1337!'));

app.get('/', (req, res) => res.send('Hello World!'));

server.on('connection', (socket) => {
	const socketId = socket.remoteAddress + ':' + socket.remotePort;
	sockets[socketId] = socket;
	console.log("Open socket", socketId);
	socket.on('close', () => {
		console.log("Close socket", socketId);
		delete sockets[socketId];
	});
});

// Example:
makeRequest('GET', 'http://localhost:1337').then(function (response, status, hdr) {
	console.log("If the connection is not closed here, there is probably an issue...");
	console.log(response, status, hdr);
}).catch(function (e) {
	console.error('Augh, there was an error!', e);
}).then(function() {
	return new Promise((resolve) => {
		setTimeout(() => {
			console.log("\nFinished. Closing server");
			server.close();
			resolve();
		}, 3000);
	});
});

On Linux, the connection is opened and closed almost instantly but there is a lag once again in Windows.

Solution

What would be the best way to solve this? We could put a delay in checking the open connections for Windows? Or is there another solution I am missing?

Environment Setup

• Server
  • parse-server version (Be specific! Don't say 'latest'.) : 2.6.5
  • Operating System: Windows 10 x64
  • Localhost or remote server? (AWS, Heroku, Azure, Digital Ocean, etc): Localhost

[montymxb]

@addisonElliott nice breakdown, thanks for digging into this! I've seen this rarely in our local/CI tests, enough that we haven't emphasized on it.

I'll get back to you with a more detailed response later, but just wanted to say I think ultimately adding at least one run in our CI using appveyor would help a ton in catching and fixing windows specific issues. I've used it personally and it's free for OSS so we wouldn't have to do too much to get that in here. We have used this in the past for the .NET SDK on top of that.

[montymxb]

So to second what I was saying before I think it would be fantastic if we add appveyor into our CI. @flovilmart if you can hookup appveyor on your end and setup a team I would be willing to put up a PR with the .travis.yml for starters.

[addisonElliott]

@montymxb I'm not familiar with appveyor myself. I'm having trouble understanding how appveyor will prevent these issues happening with running npm test. Wouldn't appveyor just be another method of running the tests?

[flovilmart]

I believe we could run in a windows environment on appVeyor. Also, this is a server project aimed to be deployed on Linux/ Mac/Unix environments, having a proper développement environment for windows is very nice to have, did you try with WLS, and proper bash instead of Cygwin? I’m not familiar with windows, so I can’t really talk about overhead or such setting up this way, but I could explore in a VM probably. I’m genuinely curious about it .)

[addisonElliott]

I got WSL downloaded and nodejs installed. It all worked fine, there were 3 errors and 9 pending but I think that is the same results I got on my Debian VPS.

@flovilmart I entirely agree that it would be great if WSL was widely adopted by Windows users, but it's going to take some time. I still think the industry standard is Cygwin. In my opinion, I think we should attempt to support Cygwin/Windows setups now.

WSL is only available if you have Windows 10 Anniversary Update or higher. I think this is a small subset of Windows users for now. Given a few years, I think WSL may be more widely adopted and in the future these problems may be a thing of the past.

**************************************************
*                    Failures                    *
**************************************************

1) Parse.User testing does not duplicate session when logging in multiple times #3451
  - Expected 2 to be 1.

2) PushController formatPushTime should format as ISO string
  - Expected '2017-09-06T22:14:01.048' to be '2017-09-06T17:14:01.048', 'No timezone'.

3) PushController Scheduling pushes in local time should preserve the push time
  - Expected '2017-09-06T22:14:01.048' to be '2017-09-06T17:14:01.048'.

**************************************************
*                    Pending                     *
**************************************************

1) Cloud Code test afterSave ignoring promise, object not found
  Temporarily disabled with xit

2) Cloud Code Logger should log a changed beforeSave indicating a change
  needs more work.....

3) parseObjectToMongoObjectForCreate a delete op
  Temporarily disabled with xit

4) Installations creating multiple devices with same device token works
  Temporarily disabled with xit

5) Installations update ios device token with duplicate token different app
  Temporarily disabled with xit

6) Parse.Object testing fetchAll User attributes get merged
  Temporarily disabled with xit

7) Parse.Query testing should handle relative times correctly
  Temporarily disabled with xit

8) Parse.Query testing should error on invalid relative time
  Temporarily disabled with xit

9) Parse.Query testing should error when using $relativeTime on non-Date field
  Temporarily disabled with xit

Executed 1547 of 1559 specs (3 FAILED) (9 PENDING) (3 SKIPPED) in 3 mins 25 secs.
npm ERR! Test failed.  See above for more details.
LAPTOP-1C8FNJ50%

[flovilmart]

This is ok, those tests have some issues when run on local machines, we need to investigate

[montymxb]

@addisonElliott appveyor provides a windows environment whereas Travis CI cannot. Having tests on windows as part of our CI would help ensure that, once we correct window related issues, we don't reintroduce them later. Better than someone finding and reporting problems when they try to run on windows themselves, kind of what it is now 😛.

[montymxb]

Just wanted to say I've come across some SO posts like this one that suggest, in certain cases, there can a significant performance loss to node.js when running on windows. I think we'll still work through it, but just something to keep in mind.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[addisonElliott]

FYI

Still looking into this for now. I have an idea why it might be slower on Windows and I think that it is because of is-mongodb-running module taking a long time due to the ps module in NodeJS.

Here is the issue I'm referencing:
neekey/ps#75

This is on my radar and I'm hoping to further my investigation. (Also, I've tried WSL but there is some limitations with running tests on it because netstat/lsof is not fully implemented and that is what some of the mongodb modules use to determine if a process is running)