Reinstate tests

[ghost]

Reinstate tests that were disabled when adding GitHub Actions to the project.

Not all tests are re-enabled yet, and some won't for a while (those testing external servers which I think don't exist anymore). For now this is based off of #241 as some tests were failing due to circular dependency issues with winston. Based off of #241 rather than #243 as the tests fail with #243 due to a missing transport.

The tests left to re-enable are those disabled in a324e16391b7375c2bd3509b72114150eb42fdfc.

[ghost]

All tests are enabled now, except 3 where external servers aren't reachable.

I don't think we want to keep https://github.com/wtsi-npg/npg_ranger/pull/245/commits/1c81314c17e2ab67ebfb875ed2075d1002a1fe91, but this is currently the only solution I've found to fix the failing tests specifically on Node 16+. The tests are those in highLevelClient.spec.js, for example: https://github.com/wtsi-npg/npg_ranger/blob/82d457b77d1477773e918c7d9a2b4b08053d26e0/test/client/highLevelClient.spec.js#L943 The server seems to fail with [ERR_HTTP_HEADERS_SENT]: Cannot remove headers after they are sent to the client

This PR also depends on #241

[jmtcsngr]

I remember not calling response.end() caused problems with the client. Maybe they changed things in node 16+, maybe it does an automatic response.end() and ours is a second call and that is why it gets the other error.

[ghost]

I haven't found anything specific related to this in Node 16 changes. Also, commenting it out works on 12 and 14 too, so maybe it was a change introduced earlier. Stepping through with the debugger on Node 16, it looks like the response.end() call is being called earlier during an await here: https://github.com/wtsi-npg/npg_ranger/blob/82d457b77d1477773e918c7d9a2b4b08053d26e0/lib/server/controller.js#L456

On node <16, response.end() isn't called at that point, which prevents the headers from already being sent.

[jmtcsngr]

It makes sense to have an error about not being able to do anything on a closed response.

What I'm worried about is that the reponse.end() was there to make sure things closed properly. I checked the docs for node 10 and node 16 and in both cases it says the method should be called. What I'm thinking is that maybe we had a case where a second call was not causing errors and they closed that "feature".

I don't see us calling reponse.end() as part of headerEncode.fullEncoding it is possible something internal calls it when consuming the buffers.

[ghost]

I think it might not be an internal call but actually just our own call happening at the wrong time (some async things I don't understand?). To clarify, I meant that by stepping through with the debugger, there are two cases:

• on node 16, when reaching the await headerEncode.fullEncoding(...), node switches to another "thread" (not sure what to call them) and goes to the block with response.end() in lib/server.js. When that block is done it returns to the code with await headerEncode.fullEncoding(..), where the headers for the response have now already been sent.
• on node <16, when reaching the await headerEncode.fullEncoding(...), node keeps going until the end of that function, and only later does it switch to the block with response.end() in lib/server.js.

So in both cases, we execute the same code, but not in the same order.