search

libp2p / js-libp2p-tcp

JavaScript implementation of the TCP module that libp2p uses that implements the interface-transport spec
https://libp2p.io
Other
76 stars 40 forks source link

Use different port in max-connections-close test #248

Closed dapplion closed 1 year ago

dapplion commented 1 year ago

Release CI is broken on this test https://github.com/libp2p/js-libp2p-tcp/actions/runs/3971032097/jobs/6814690159

  1) close server on maxConnections
       reject dial of connection above closeAbove:
     Socket[1] connect ECONNREFUSED ::1:9900
  Error: connect ECONNREFUSED ::1:9900
      at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1487:16)

I am not able to reproduce the error locally. The test is supposed to allow the first 3 connections (socket[1], socket[2], socket[3]). However, the first attempted socket fails to connect as apparently the server is not listening. The server should listen here and this promise resolve only when sockets can connect.

https://github.com/libp2p/js-libp2p-tcp/blob/07a03ac4cdb645393b6d7a5ec88059da034a9b45/test/max-connections-close.spec.ts#L28

This PR does not provide any guaranteed solution but just changes the port to ensure test max-connections.spec.ts does not conflict with max-connections-close.spec.ts.

Why is test CI only failing on release?

mpetrunic commented 1 year ago

It seems like it's just flaky test as it passes in other matrix jobs and in future runs. Afaik mocha runs tests sequentially by default so they shouldn't clash

achingbrain commented 1 year ago

Thanks for opening this but I don't think this is the right solution because the error message is that the connection was refused, not that the port was in use.

This PR likely passed CI because it didn't trigger the problem originally observed.

I've opened https://github.com/libp2p/js-libp2p-tcp/issues/250 for further investigation.