fix: some others flaky tests

[robertsLando]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 80.00000% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 79.45%. Comparing base (2b75186) to head (e8462b3).

Files	Patch %	Lines
src/lib/timers.ts	71.42%	0 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1808      +/-   ##
==========================================
+ Coverage   79.40%   79.45%   +0.04%     
==========================================
  Files          24       25       +1     
  Lines        1423     1426       +3     
  Branches      332      332              
==========================================
+ Hits         1130     1133       +3     
  Misses        212      212              
  Partials       81       81              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[robertsLando]

@mcollina I recently started to randomly get this error on tests: https://github.com/mqttjs/MQTT.js/actions/runs/8143942807/job/22256983402?pr=1808#step:6:188

  'test did not finish before its parent and was cancelled'

Any clue?

[robertsLando]

@mcollina It seems this happens very randomly, initially I thought that it was caused by a leaked handler but I have been able to reproduce it locally by running tests on a single file and it fails after few seconds (when it happens). It makes me thinking about a possible race condition in tests but I have no clue where to look/debug

[mcollina]

lgtm

[robertsLando]

@mcollina the issue is not fixed btw, I still get that error randomly...

[mcollina]

The tests are not properly isolated due to them running against the same broker and in the same process. porting them to tap or node:test would fix the latter, and a major refactoring would fix the former. Otherwise this is a goose chase.

[robertsLando]

The tests are not properly isolated due to them running against the same broker and in the same process. porting them to tap or node:test would fix the latter

Maybe you forgot about: #1697 that exaxtly did such things

1. Each test is using a different broker on a different port:

This is the method used:

MQTT.js/test/helpers/port_list.ts

Line 6 in e8462b3

export default function getPorts(i = 0) {

Then each file requires a different port like:

MQTT.js/test/secure_client.ts

Line 11 in e8462b3

const ports = getPorts(5)

3. Tests are already using node:test suite:

MQTT.js/test/runTests.ts

Line 2 in e8462b3

import { run } from 'node:test'

As you can see also the test suite are run in parallel

[robertsLando]

@mcollina Ping

[robertsLando]

@mcollina I will merge this for now as it makes some improvements I wish to include on next release, if you find some time to check the code and find a possible reason why that error happens let me know 🙏🏼