Fix flaky test

[andyleejordan]

I believe this test was flaky due to race conditions caused by the events. Awaiting promises instead should eliminate those races.

[andyleejordan]

Let's see if they all pass.

[JustinGrote]

I believe this test was flaky due to race conditions caused by the events. Awaiting promises instead should eliminate those races.

Events are so hard to test. I think it might still race flake so we can try using the done() callback in the event to make sure it doesn't end until all the assertions are complete. I'll take a look

[andyleejordan]

I did also increase the timeout in the release PR while I was adding a skip:

Binary Modules
  ✔ Debugs a binary module script (11974ms)
  ✔ Stops at a binary module breakpoint (10853ms)

But IIRC it was set to 20m000ms and they're coming in around 10,000ms.

Anyway, I'm down to merge this and let upcoming work test the flakiness. They passed on the first try this time!

[andyleejordan]

Re-running checks to see if they pass again.

[andyleejordan]

Events are so hard to test. I think it might still race flake so we can try using the done() callback in the event to make sure it doesn't end until all the assertions are complete. I'll take a look

That's essentially what the Promise does. By doing the assignment within the event as a resolution to the promise, the latter test code which needs the value waits for that resolved promise (and thus assignment) to actually happen (which is in the background asynchronously via the event). So instead of racing the event to its completion, the test code awaits for that logic to happen.

[SeeminglyScience]

LGTM!

[andyleejordan]

2 failing

1. RunCode feature
   "before all" hook: ensureEditorServicesIsConnected for "Creates the launch config":
   Error: Timeout of 10000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/Users/runner/work/1/s/vscode-powershell/out/test/features/RunCode.test.js)
   at listOnTimeout (node:internal/timers:559:17)
   at process.processTimers (node:internal/timers:502:7)

2. ISE compatibility feature
   "before all" hook in "ISE compatibility feature":
   Error: The extension 'ms-vscode.powershell' is already registered.
   at xs.registerExternalExtension (out/main.js:87:30059)
   at Object.registerExternalExtension (out/main.js:91:1370)
   at Object.ensureEditorServicesIsConnected (out/test/utils.js:85:33)
   at async Context. (out/test/features/ISECompatibility.test.js:40:9)

Grr, we need more timeout for the before.

[andyleejordan]

All right, I think we hit bingo!

[JustinGrote]

Events are so hard to test. I think it might still race flake so we can try using the done() callback in the event to make sure it doesn't end until all the assertions are complete. I'll take a look

That's essentially what the Promise does. By doing the assignment within the event as a resolution to the promise, the latter test code which needs the value waits for that resolved promise (and thus assignment) to actually happen (which is in the background asynchronously via the event). So instead of racing the event to its completion, the test code awaits for that logic to happen.

Makes sense, I have some other ideas but LGTM! Thanks.