Standardise conditional testing fix

[tegefaulkes]

Description

This PR standardises how conditional testing with testIf and describeIf is used with new reified booleans.

Issues Fixed

• Fixes Standardise Conditional Testing Utilties #434
• Related Reusable (and exportable) Test Cases - grouping check and integration stage tests #435

Final checklist

• Domain specific tests
• Full tests
• Lint fixed
• Squash and rebased
• Sanity check the final build

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[CMCDragonkai]

Can you make sure global is changed to globalThis?

[CMCDragonkai]

You can use https://eslint.org/docs/latest/rules/no-restricted-globals to add a restriction to global. And provide a replacement for globalThis. I think it can be autofixed then.

[CMCDragonkai]

I think the PR still needs a task list to be ticked off. The PR can have tasks that indicate pending things to be done that isn't about the issue.

[tegefaulkes]

I didn't include the task list here since this PR leans more to review than drafting out the problem. I'll add it back in but any todos are covered by the review comments right now.

[CMCDragonkai]

I just did a quick check, and it is actually:

import * as testUtils from '../../utils';

That is the most common.

Not:

import * as testsUtils from '../../utils';

So I guess we keep it as testUtils for now?

This is because I can see there are other similar test.+Utils.

Hard to remember cause the directory is tests not test.

[tegefaulkes]

Changed it to testUtils now.

[tegefaulkes]

I'm re-triggering agent/service and nat tests in CI. Seems like they failed due to timeouts. It's just 3 tests in total so I think we're good to merge here.

[tegefaulkes]

They timed out again.

 ● nodesCrossSignClaim › fails after receiving undefined singly signed claim

    thrown: "Exceeded timeout of 20000 ms for a hook.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      31 |   let localId: NodeId;
      32 |   let remoteId: NodeId;
    > 33 |   beforeEach(async () => {
         |   ^
      34 |     dataDir = await fs.promises.mkdtemp(
      35 |       path.join(os.tmpdir(), 'polykey-test-'),
      36 |     );

      at tests/agent/service/nodesCrossSignClaim.test.ts:33:3
      at Object.<anonymous> (tests/agent/service/nodesCrossSignClaim.test.ts:20:1)
      
  ● endpoint independent NAT traversal › node1 behind EIM NAT connects to node2 behind EIM NAT via seed node
    thrown: "Exceeded timeout of 40000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."
      292 |     globalThis.defaultTimeout * 2,
      293 |   );
    > 294 |   testUtils.testIf(supportsNatTesting)(
          |                                       ^
      295 |     'node1 behind EIM NAT connects to node2 behind EIM NAT via seed node',
      296 |     async () => {
      297 |       const {
      at tests/nat/endpointIndependentNAT.test.ts:294:39
      at Object.<anonymous> (tests/nat/endpointIndependentNAT.test.ts:15:1)
      

I don't think there's a problem with the tests themselves. I know the agent/service test is passing locally. Barring a deeper dive into the problem I think we can only increase the timeout for this.

Want me to look into it now or merge now and fix later?

[CMCDragonkai]

Ok leaving the time outs to a later PRs.