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

mssql.close() still blocking mocha test suite exit #735

Closed
missingmeow opened this issue Oct 23, 2018 · 9 comments
Closed

mssql.close() still blocking mocha test suite exit #735

missingmeow opened this issue Oct 23, 2018 · 9 comments

Comments

@missingmeow
Copy link

I have met the same question like this ladjs/supertest#437 (comment),.

So I need to close all the connections after I call a unit test, I have tried mssql.close() but it doesn't work.

Has anybody met this before and how do you fix it?

the mssql code i use look like this:

const conn = new mssql.ConnectionPool(config, (err) => {
  const ps = new mssql.PreparedStatement(conn);
  ... // deal with ps.input or something
  ps.prepare(sql, (err) => {
    ps.execute(obj, (err, record) => {
      ... // deal with the record
      ps.unprepared((err) => {})
    }
  }
}
@willmorgan
Copy link
Collaborator

What about .close doesn't work?

@missingmeow
Copy link
Author

I don't quite understand what you mean.

I have tried mssql.close() and it doesn't work.

btw, package.json looks like:

"dependencies": {
  "config-lite": "^2.1.0",
  "cross-env": "^5.2.0",
  "express": "^4.16.3",
  "mssql": "^4.2.1",
  "mocha": "^5.2.0",
  "moment": "^2.22.2",
  "morgan": "^1.9.1",
  "rotating-file-stream": "^1.3.9",
  "should": "^13.2.3",
  "supertest": "^3.3.0",
  "istanbul": "^0.4.5"
},
"scripts": {
  "test": "cross-env NODE_ENV=development istanbul cover ./node_modules/mocha/bin/_mocha"
}

@willmorgan
Copy link
Collaborator

Thanks for the package.json example. If you could perhaps post an error message, that would be helpful - "it doesn't work" is quite vague and as this is a free project, time is scarce.

https://developer.mozilla.org/en-US/docs/Mozilla/QA/Bug_writing_guidelines#Writing_precise_steps_to_reproduce

@missingmeow
Copy link
Author

missingmeow commented Oct 23, 2018

My apologies.

I have written a sample test like this:

const should = require('should');
const mssql = require('mssql');
const config = require('config-lite')(__dirname).mssql;

function execute(sql, callBack) {
    const connPool = new mssql.ConnectionPool(config, (err) => {
        const ps = new mssql.PreparedStatement(connPool);
        ps.prepare(sql, (err) => {
            if (err) {
                console.log(err);
            }
           ps.execute({}, (err, recordset) => {
                if (err) {
                    console.log(err);
                }
                callBack(err, recordset);
                ps.unprepare((err) => {
                    if (err) {
                        console.log(err);
                    }
                });
            });
        });
    });
}
describe('mssql test', () => {
    after(() => {
        mssql.close();
        console.log('after all run out');
    })
    it('just return someting and go on', (done) => {
        execute('select * from TestForNode', (err, record) => {
            done();
        })
    });
});

Then run the command npm test and the output is as follows:

$ npm test
> cross-env NODE_ENV=development istanbul cover ./node_modules/mocha/bin/_mocha
  mssql test
    √ just return someting and go on (633m)
after all run out
  1 passing (649ms)

It just waiting there forever, it won't exit unless you press Ctrl + C.

If I don't call the execute function, just done(), the output is as follows:

$ npm test
> cross-env NODE_ENV=development istanbul cover ./node_modules/mocha/bin/_mocha
  mssql test
    √ just return someting and go on
after all run out
  1 passing (13ms)
=============== Coverage summary ======================
Statements   : 100% ( 3/3 )
Branches     : 100% ( 0/0 )
Functions    : 100% ( 0/0 )
Lines        : 100% ( 3/3 )
==================================================

@willmorgan
Copy link
Collaborator

This is some new Mocha functionality. You can run mocha with --exit as a quick fix.

In the interim, try something like this:

    after(async () => {
        await mssql.close();
        console.log('after all run out');
    })

@missingmeow
Copy link
Author

Thank you very much.

First, I have tried to run mocha --exit with or without await, they both work, everything is OK.

Then, I've removed the argument --exit with await in the code, it still won't exit as above.

Thanks again.

@willmorgan
Copy link
Collaborator

@Sondragon OK, great, thanks for the update.

I'll amend the issue title and look at putting this somewhere on the backlog to get resolved.

@willmorgan willmorgan changed the title how to close all the connctions? mssql.close() still blocking mocha test suite exit Oct 24, 2018
@dhensby
Copy link
Collaborator

dhensby commented Oct 24, 2018

The problem is that there's no global connection to close (which is what mssql.close() is doing in this case).

You need to change your code to close the connection pool:

const should = require('should');
const mssql = require('mssql');
const config = require('config-lite')(__dirname).mssql;

function execute(sql, callBack) {
    const connPool = new mssql.ConnectionPool(config, (err) => {
        const ps = new mssql.PreparedStatement(connPool);
        ps.prepare(sql, (err) => {
            if (err) {
                console.log(err);
            }
           ps.execute({}, (err, recordset) => {
                if (err) {
                    console.log(err);
                }
                callBack(err, recordset);
                ps.unprepare((err) => {
                    if (err) {
                        console.log(err);
                    }
+                    connPool.close();
                });
            });
        });
    });
}
describe('mssql test', () => {
-    after(() => {
-        mssql.close();
-        console.log('after all run out');
-    })
    it('just return someting and go on', (done) => {
        execute('select * from TestForNode', (err, record) => {
            done();
        })
    });
});

@dhensby dhensby closed this as completed Oct 24, 2018
@dhensby
Copy link
Collaborator

dhensby commented Oct 24, 2018

You should investigate having a shared pool instead which you pass around to your tests and then close in the after callback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants