test(ec2): fix flaky tests regarding child node updates.

[Hweinstock]

Problem

The issue is here:

aws-toolkit-vscode/packages/core/src/awsService/ec2/explorer/ec2ParentNode.ts

Lines 55 to 64 in 9eea1f3

	private updatePendingNodes() {
	this.pollingSet.forEach(async (instanceId) => {
	const childNode = this.ec2InstanceNodes.get(instanceId)!
	await childNode.updateStatus()
	if (!childNode.isPending()) {
	this.pollingSet.delete(instanceId)
	await childNode.refreshNode()
	}
	})
	}

This fires off multiple asynchronous calls on each child, with the function returning before any are resolved. Thus, the child node could be deleted from the set after this function returned (potentially when another piece of code is using it). This leads to undefined errors within that function.

Solution

Refactor the code to make use of a for...of with await pattern.

---

License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

[justinmk3]

nice catch. maybe we should ban .forEach, it's pretty useless

does this close an issue or it's just something you noticed?

[Hweinstock]

Yeah, I set up an issue tracking this as part of the reliability work when I saw it caused a CI file on another branch. Luckily it seems like a quick fix.

Also, I would agree about the forEach. It lets you give it an async function but the behavior isn't what you'd expect. Could also write/reuse some of the asyncCollection utilities to make an async safe version.