User story
As someone reviewing or merging a PR, I want the worker integration tests to pass or fail based on the code, so that a red build means something.
Details
The worker integration job fails intermittently, and which of the three Node versions fails moves around from run to run. On PR #1500 we saw it go 22.20 fail / 24.18 pass / 26.5 pass, then the reverse, then back again - with an empty commit between the last two, so the code was identical. The failure looks like Error: listen EADDRINUSE: address already in use :::2492, followed by a test file whose tests all sit pending until ava's two minute timeout kills the job.
The cause is that the tests pick a port at random between 2000 and 3000 and start a server on it, without checking whether that number is already in use. integration-tests/worker/src/init.ts does this once for the mock Lightning server and again for every worker, and one test file starts a worker a dozen times. With that many draws from a thousand numbers, two of them landing on the same port is likely enough to happen every few runs. When the second server can't bind, the setup hook never finishes, so nothing in that file ever runs.
It's worth noting that three test files also hardcode the same Lightning port (4321). That's harmless today because ava runs the files one at a time, but it's a trap for anyone who later turns on file concurrency.
Implementation notes
Because the files run serially and each one is its own process, handing out ports from a counter instead of guessing is enough to remove the collision - something like starting at 4400 and incrementing. The random helper is only called in two places, so changing it in one spot covers both. Worth picking a base that doesn't overlap the 3000s, which the server tests already use.
The more thorough option is to bind to port 0 and let the operating system pick a free one, then read back what it assigned. That's a bigger change, because the mock Lightning server has to be listening before a worker can be told where to connect, which means touching the setup in all six test files. Probably not worth it unless we want file concurrency later.
While in there, the duplicated 4321 could be made distinct per file so the trap goes away.
Release notes
None - test infrastructure only.
Tests
Running the worker integration suite repeatedly should stop producing the occasional hung file. A quick sanity check is that no two servers in a single run are asked for the same port.
User story
As someone reviewing or merging a PR, I want the worker integration tests to pass or fail based on the code, so that a red build means something.
Details
The worker integration job fails intermittently, and which of the three Node versions fails moves around from run to run. On PR #1500 we saw it go 22.20 fail / 24.18 pass / 26.5 pass, then the reverse, then back again - with an empty commit between the last two, so the code was identical. The failure looks like
Error: listen EADDRINUSE: address already in use :::2492, followed by a test file whose tests all sit pending until ava's two minute timeout kills the job.The cause is that the tests pick a port at random between 2000 and 3000 and start a server on it, without checking whether that number is already in use.
integration-tests/worker/src/init.tsdoes this once for the mock Lightning server and again for every worker, and one test file starts a worker a dozen times. With that many draws from a thousand numbers, two of them landing on the same port is likely enough to happen every few runs. When the second server can't bind, the setup hook never finishes, so nothing in that file ever runs.It's worth noting that three test files also hardcode the same Lightning port (4321). That's harmless today because ava runs the files one at a time, but it's a trap for anyone who later turns on file concurrency.
Implementation notes
Because the files run serially and each one is its own process, handing out ports from a counter instead of guessing is enough to remove the collision - something like starting at 4400 and incrementing. The random helper is only called in two places, so changing it in one spot covers both. Worth picking a base that doesn't overlap the 3000s, which the server tests already use.
The more thorough option is to bind to port 0 and let the operating system pick a free one, then read back what it assigned. That's a bigger change, because the mock Lightning server has to be listening before a worker can be told where to connect, which means touching the setup in all six test files. Probably not worth it unless we want file concurrency later.
While in there, the duplicated 4321 could be made distinct per file so the trap goes away.
Release notes
None - test infrastructure only.
Tests
Running the worker integration suite repeatedly should stop producing the occasional hung file. A quick sanity check is that no two servers in a single run are asked for the same port.