Federation background delivery process test fail

[gregid]

Hi, going through your code as ActivityPub learning exercise as this is the nicest ActivityPub code I could find on the web!

Back to the point: I got 2 tests failing. Going through documentation you've warned about possible http-signature issue, but
I didn't get the InvalidHeaderError, so I am raising this as an issue.

The tests were run on Windows 10, Node: v14.5.0, NPM: 6.14.5.

1) federation background delivery process backs off repeated attempts
  - Expected spy deliver to have been called 4 times. It was called 3 times.

2) federation background delivery process can restart delivery while retries are pending
  - Expected spy deliver to have been called 4 times. It was called 2 times.
  - Expected spy deliver to have been called 6 times. It was called 5 times.
  - Expected $[2] = 'https://mocked.com/bob/inbox' to equal 'https://ignore.com/sally/inbox'.

[gregid]

I updated my node to v14.16.0 but the problem persists

[wmurphyrd]

Thanks for the kind words. These two tests use timeouts, so they are prone to false positives. They may pass if you re-run the tests. The tests could be improved by replacing the timeouts with a combination of clock mocking (to run out delivery delay timers) and process.nextTick (to allow promises to resolve)

[gregid]

Thanks for the reply - I suspected it must be timeout related and tried different timeout values but haven't been successful in finding the right ones to pass the test - but I am happy to hear it is more of an imprecise test issue rather then a code issue.

[gregid]

I decided to give it a try few more times and I think I've found a sweet spot (for my machine at least). Giving 20ms per each call makes all the tests pass (4 calls: 80, 5 calls: 100, 6 calls: 120). It still passes the tests in 50% of time but at least I have seen all green.

Still seeing that deliveryDequeue().waitUntil.getTime() - Date.now() is different per each call I don't see how it could be tested to be universally correct (unless the clock mocking somehow makes it work).