Tests can hang indefinitely when connection resources aren’t cleaned up after assertion failures

[HappyZombies]

TL;DR: The tests don’t reliably clean up after themselves on failures, failed assertions can leave connections, pools, or prepared statements open, causing the process to hang. The poku testing module doesn't seem to have an option to "fix" these tests, so certain tests must be manually fixed.

Hey there! I wrote an issue a few days ago and found time to sit down and look into what the problem was, and I believe I have found it!

While running the mysql2 test suite locally, I noticed that several unit tests can hang randomly if a connection, pool, or prepared statement isn’t properly closed when a test fails.

I initially discovered this because my test database wasn’t named test (I used nodetest). That caused some assertions to fail, since some of the checks are hardcoded (for example, here) but instead of the suite exiting cleanly, it would hang indefinitely.

After digging, I found that some test files wrap test() calls in async IIFEs and/or place cleanup code (e.g. connection.end() or pool.end()) in a stop that can occur after a failing assert. So, when the assertion throws, that teardown line never runs, leaving sockets or prepared statements open. I think that Poku waits for the event loop to drain, but the leaked handle keeps it alive forever.

I have ran this problem into the following files, but I have seen it/sure that there are way more.

• https://github.com/sidorares/node-mysql2/blob/master/test/esm/integration/connection/test-column-inspect.test.mjs
• https://github.com/sidorares/node-mysql2/blob/master/test/esm/unit/check-extensions.test.mjs

Note that it's not always something that is wrapped in an async IIFEs, but a majority of the hangs are.

Integration test test-column-inspect.test.mjs is the easiest place to see this occur -- simply make that last assert fail in any way (add a typo/bad field or create a database who's name isn't test) and the test won't ever continue/close/finish do the hanging connection.

Or just run this simple unit test some test directory.

import { test, assert } from 'poku';
import util from 'node:util';
import { createRequire } from 'node:module';

const require = createRequire(import.meta.url);
const common = require('../../../common.test.cjs');

(async () => {
  const connection = common.createConnection().promise();

  test(async () => {
    const [, columns] = await connection.query(
      'SELECT SCHEMA_NAME AS Database_Name FROM INFORMATION_SCHEMA.SCHEMATA;'
    );
    const inspectResults = util.inspect(columns[1]);

    // These will not match, obviously, so it will throw...BUT the test will never resolve
    assert.deepEqual(
      inspectResults,
      util.inspect({
        catalog: 'def',
        schema: 'test',
        name: 'decimal13_10',
        orgName: 'decimal13_10',
        table: 'test_fields2',
        orgTable: 'test_fields2',
        characterSet: 63,
        encoding: 'binary',
        columnLength: 14,
        type: 246,
        flags: ['NOT NULL'],
        decimals: 10,
        typeName: 'NEWDECIMAL',
      }),
      'should show detailed description when depth is < 1'
    );
  });

  await connection.end();
})();

I hate to tag you, but @wellwelwel -- does your test package of poku offer a command line for "noExit" like Mocha does right? https://mochajs.org/next/running/cli/#--exit -- but is this even equivalent/the same?

But then again, idk if your noExit option behaves the same or different to the mocha one.

Anyways, what this means that if a test leaks a resource (MySQL connection, pool, or prepared statement) the process never exits naturally. The only way to ensure clean termination is to fix the tests themselves by guaranteeing teardown (connection.end(), pool.end(), stmt.close(), etc.) runs even on failure.

I am willing to find sometime to fix this, or unless there's a way to get poku to handle these hangs for us (I am not THAT familiar with the testing framework but I did spend all day on the docs today looking, lol). But point it... I am making the issue to discuss so we can see what would be the ideal thing here to fix these.

[wellwelwel]

I hate to tag you, but @wellwelwel

Hey, @HappyZombies! Don't worry about tagging me 😂

Suppose we used Mocha for testing, the --exit option wouldn't be a viable approach for MySQL2. What happens is that after mapping the tests and running all the test methods internally, Mocha will force a process.exit.

This leads to false positives where a test might pass, but in reality it would be holding up the process indefinitely outside the test context.

Note

does your test package of poku offer a command line for "noExit" like Mocha does right?

Regarding Poku's noExit option, it has a different purpose and is only used for advanced implementations to use the test runner as an API. You can see an example at https://poku.io/docs/examples/cases/complexity-no-exit. Also, it's only for API usage (not CLI).

[HappyZombies]

Sounds good @wellwelwel ! Thanks for the clarification, In that case since the mocha flag wont work in the case, I'm guessing the solution here is that the unit tests should indeed then be updated to cleanup so resources don't hang then. Correct? If so I'll try to make this my first PR lol