refactor(pollingset): make bad state impossible #6396

## Problem
Right now `PollingSet` exposes both the `add` and the `start` methods.
The `start` adds the item and starts the timer, while the `add` only
adds the item.

This allows a consumer to create a non-empty `PollingSet` where the
timer never started.

## Solution
- override the add method so that a consumer is forced to start the
timer when adding items.

Author: Hweinstock

packages/core/src/awsService/apprunner/explorer/apprunnerNode.ts

@@ -93,7 +93,7 @@ export class AppRunnerNode extends AWSTreeNodeBase {
     }
 
     public startPollingNode(id: string): void {
-        this.pollingSet.start(id)
+        this.pollingSet.add(id)
     }
 
     public stopPollingNode(id: string): void {

packages/core/src/awsService/ec2/explorer/ec2ParentNode.ts

@@ -45,7 +45,7 @@ export class Ec2ParentNode extends AWSTreeNodeBase {
         if (!this.ec2InstanceNodes.has(instanceId)) {
             throw new Error(`Attempt to track ec2 node ${instanceId} that isn't a child`)
         }
-        this.pollingSet.start(instanceId)
+        this.pollingSet.add(instanceId)
     }
 
     public async updateChildren(): Promise<void> {

packages/core/src/shared/utilities/pollingSet.ts

@@ -44,10 +44,11 @@ export class PollingSet<T> extends Set<T> {
             this.clearTimer()
         }
     }
-    // TODO(hkobew): Overwrite the add method instead of adding seperate method. If we add item to set, timer should always start.
-    public start(id: T): void {
-        this.add(id)
+
+    public override add(id: T) {
+        super.add(id)
         this.pollTimer = this.pollTimer ?? globals.clock.setInterval(() => this.poll(), this.interval)
+        return this
     }
 
     public override clear(): void {

packages/core/src/shared/utilities/processUtils.ts

@@ -119,7 +119,7 @@ export class ChildProcessTracker {
     public add(childProcess: ChildProcess) {
         const pid = childProcess.pid()
         this.#processByPid.set(pid, childProcess)
-        this.#pids.start(pid)
+        this.#pids.add(pid)
     }
 
     public delete(childProcessId: number) {

packages/core/src/test/awsService/ec2/activation.test.ts

@@ -19,7 +19,7 @@ describe('ec2 activation', function () {
         const testPartition = 'test-partition'
         // Don't want to be polling here, that is tested in ../ec2ParentNode.test.ts
         // disabled here for convenience (avoiding race conditions with timeout)
-        sinon.stub(PollingSet.prototype, 'start')
+        sinon.stub(PollingSet.prototype, 'add')
         const testClient = new Ec2Client(testRegion)
         const parentNode = new Ec2ParentNode(testRegion, testPartition, new Ec2Client(testRegion))
         testNode = new Ec2InstanceNode(parentNode, testClient, testRegion, testPartition, {

packages/core/src/test/awsService/ec2/explorer/ec2InstanceNode.test.ts

@@ -35,7 +35,7 @@ describe('ec2InstanceNode', function () {
         sinon.stub(Ec2InstanceNode.prototype, 'updateStatus')
         // Don't want to be polling here, that is tested in ../ec2ParentNode.test.ts
         // disabled here for convenience (avoiding race conditions with timeout)
-        sinon.stub(PollingSet.prototype, 'start')
+        sinon.stub(PollingSet.prototype, 'add')
         const testClient = new Ec2Client('')
         const testParentNode = new Ec2ParentNode(testRegion, testPartition, testClient)
         testNode = new Ec2InstanceNode(testParentNode, testClient, 'testRegion', 'testPartition', testInstance)

packages/core/src/test/shared/utilities/pollingSet.test.ts

@@ -56,7 +56,7 @@ describe('pollingSet', function () {
         const action = sinon.spy()
         pollingSet = new PollingSet(10, action)
         sinon.assert.notCalled(action)
-        pollingSet.start('item')
+        pollingSet.add('item')
 
         await clock.tickAsync(9)
         sinon.assert.notCalled(action)
@@ -66,7 +66,7 @@ describe('pollingSet', function () {
 
     it('stops timer once polling set is empty', async function () {
         const pollingSet = new PollingSet(10, () => {})
-        pollingSet.start('1')
+        pollingSet.add('1')
         pollingSet.add('2')
 
         const clearStub = sinon.stub(pollingSet, 'clearTimer')
@@ -90,7 +90,7 @@ describe('pollingSet', function () {
     it('runs action once per interval', async function () {
         const action = sinon.spy()
         pollingSet = new PollingSet(10, action)
-        pollingSet.start('1')
+        pollingSet.add('1')
         pollingSet.add('2')
 
         sinon.assert.callCount(action, 0)