Wait for ICE gathering completion before requesting video track + create file e2e test fix (#5193)

* Test main e2e

* Create projects separately in home page tests

I think creating them in Promise.all was introducing nondeterminism and
making tests flaky.

* Query the homepage projects in an order-insensitive way

* Wait for ICE candidate gathering to complete before requesting video track

* Update src/lang/std/engineConnection.ts

Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

* Fix create file e2e failure

* Yarn fmt

* Fix typo: s/that/this

yarn tsc was failing with this error:

```
src/lang/std/engineConnection.ts:1285:7 - error TS2304: Cannot find name 'that'.

1285       that.triggeredStart = false
           ~~~~
```

* Fix up revolve tests

* Turn off 3 flaky Windows tests

* Fix tags

---------

Co-authored-by: Adam Chalmers <adam.chalmers@zoo.dev>
Co-authored-by: Jonathan Tran <jonnytran@gmail.com>

e2e/playwright/command-bar-tests.spec.ts

@@ -127,10 +127,10 @@ test.describe('Command bar tests', () => {
     await expect(commandLevelArgButton).toHaveText('level: project')
   })
 
-  test('Command bar keybinding works from code editor and can change a setting', async ({
+  test(
-    page,
+    'Command bar keybinding works from code editor and can change a setting',
-    homePage,
+    { tag: ['@skipWin'] },
-  }) => {
+    async ({ page, homePage }) => {
       await page.setBodyDimensions({ width: 1200, height: 500 })
       await homePage.goToModelingScene()
 
@@ -162,10 +162,9 @@ test.describe('Command bar tests', () => {
       await page.keyboard.press('ArrowDown')
       await page.keyboard.press('ArrowDown')
       await page.keyboard.press('ArrowDown')
-    await expect(page.getByRole('option', { name: 'system' })).toHaveAttribute(
+      await expect(
-      'data-headlessui-state',
+        page.getByRole('option', { name: 'system' })
-      'active'
+      ).toHaveAttribute('data-headlessui-state', 'active')
-    )
       await page.keyboard.press('Enter')
 
       // Check the toast appeared
@@ -174,7 +173,8 @@ test.describe('Command bar tests', () => {
       ).toBeVisible()
       // Check that the theme changed
       await expect(page.locator('body')).not.toHaveClass(`body-bg dark`)
-  })
+    }
+  )
 
   test('Can extrude from the command bar', async ({ page, homePage }) => {
     await page.addInitScript(async () => {

e2e/playwright/editor-tests.spec.ts

@@ -966,10 +966,10 @@ test.describe('Editor tests', () => {
   |> close(%)`)
   })
 
-  test('Can undo a sketch modification with ctrl+z', async ({
+  test(
-    page,
+    'Can undo a sketch modification with ctrl+z',
-    homePage,
+    { tag: ['@skipWin'] },
-  }) => {
+    async ({ page, homePage }) => {
       const u = await getUtils(page)
       await page.addInitScript(async () => {
         localStorage.setItem(
@@ -1113,7 +1113,8 @@ test.describe('Editor tests', () => {
   |> tangentialArcTo([24.95, -0.38], %)
   |> close(%)
   |> extrude(5, %)`)
-  })
+    }
+  )
 
   test.fixme(
     `Can use the import stdlib function on a local OBJ file`,

e2e/playwright/file-tree.spec.ts

@@ -19,7 +19,7 @@ test.describe('integrations tests', () => {
         )
       })
 
-      const [clickObj] = await scene.makeMouseHelpers(600, 300)
+      const [clickObj] = await scene.makeMouseHelpers(726, 272)
 
       await test.step('setup test', async () => {
         await homePage.expectState({
@@ -61,6 +61,7 @@ test.describe('integrations tests', () => {
       })
       await test.step('setup for next assertion', async () => {
         await toolbar.openFile('main.kcl')
+        await scene.waitForExecutionDone()
         await clickObj()
         await scene.moveNoWhere()
         await editor.expectState({

e2e/playwright/fixtures/homePageFixture.ts

@@ -89,18 +89,11 @@ export class HomePageFixture {
    * Maybe there a good sanity check we can do each time?
    */
   expectState = async (expectedState: HomePageState) => {
-    await expect
+    await expect.poll(this._serialiseSortBy).toEqual(expectedState.sortBy)
-      .poll(async () => {
+
-        const [projectCards, sortBy] = await Promise.all([
+    for (const projectCard of expectedState.projectCards) {
-          this._serialiseProjectCards(),
+      await expect.poll(this._serialiseProjectCards).toContainEqual(projectCard)
-          this._serialiseSortBy(),
-        ])
-        return {
-          projectCards,
-          sortBy,
     }
-      })
-      .toEqual(expectedState)
   }
 
   createAndGoToProject = async (projectTitle = 'project-$nnn') => {

e2e/playwright/fixtures/toolbarFixture.ts

@@ -62,7 +62,9 @@ export class ToolbarFixture {
     this.filePane = page.locator('#files-pane')
     this.featureTreePane = page.locator('#feature-tree-pane')
     this.fileCreateToast = page.getByText('Successfully created')
-    this.exeIndicator = page.getByTestId('model-state-indicator-execution-done')
+    this.exeIndicator = page.getByTestId(
+      'model-state-indicator-receive-reliable'
+    )
   }
 
   get logoLink() {

e2e/playwright/point-click.spec.ts

@@ -2273,10 +2273,6 @@ radius = 8.69
     const lineCodeToSelection = `|> angledLine([0, 202.6], %, $rectangleSegmentA001)`
     await page.getByText(lineCodeToSelection).click()
     await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
 
     const newCodeToFind = `revolve001 = revolve({angle = 360, axis = getOppositeEdge(rectangleSegmentA001)}, sketch002) `
     expect(editor.expectEditor.toContain(newCodeToFind)).toBeTruthy()
@@ -2328,10 +2324,6 @@ radius = 8.69
     const lineCodeToSelection = `|> xLine(2.6, %)`
     await page.getByText(lineCodeToSelection).click()
     await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
-    await cmdBar.progressCmdBar()
 
     const newCodeToFind = `revolve001 = revolve({ angle = 360, axis = seg01 }, sketch003)`
     expect(editor.expectEditor.toContain(newCodeToFind)).toBeTruthy()

e2e/playwright/projects.spec.ts

@@ -1527,12 +1527,10 @@ test(
   { tag: '@electron' },
   async ({ context, page, cmdBar, homePage }, testInfo) => {
     await context.folderSetupFn(async (dir) => {
-      await Promise.all([
+      await fsp.mkdir(path.join(dir, 'router-template-slate'), {
-        fsp.mkdir(path.join(dir, 'router-template-slate'), { recursive: true }),
+        recursive: true,
-        fsp.mkdir(path.join(dir, 'bracket'), { recursive: true }),
+      })
-      ])
+      await fsp.copyFile(
-      await Promise.all([
-        fsp.copyFile(
         path.join(
           'src',
           'wasm-lib',
@@ -1542,8 +1540,9 @@ test(
           'router-template-slate.kcl'
         ),
         path.join(dir, 'router-template-slate', 'main.kcl')
-        ),
+      )
-        fsp.copyFile(
+      await fsp.mkdir(path.join(dir, 'bracket'), { recursive: true })
+      await fsp.copyFile(
         path.join(
           'src',
           'wasm-lib',
@@ -1553,8 +1552,7 @@ test(
           'focusrite_scarlett_mounting_braket.kcl'
         ),
         path.join(dir, 'bracket', 'main.kcl')
-        ),
+      )
-      ])
     })
     const u = await getUtils(page)
     await page.setBodyDimensions({ width: 1200, height: 500 })

e2e/playwright/sketch-tests.spec.ts

@@ -312,7 +312,10 @@ test.describe('Sketch tests', () => {
       |> line([1.97, 2.06], %)
       |> close(%)`)
     }
-    test('code pane open at start-handles', async ({ page, homePage }) => {
+    test(
+      'code pane open at start-handles',
+      { tag: ['@skipWin'] },
+      async ({ page, homePage }) => {
         // Load the app with the code panes
         await page.addInitScript(async () => {
           localStorage.setItem(
@@ -326,9 +329,13 @@ test.describe('Sketch tests', () => {
           )
         })
         await doEditSegmentsByDraggingHandle(page, homePage, ['code'])
-    })
+      }
+    )
 
-    test('code pane closed at start-handles', async ({ page, homePage }) => {
+    test(
+      'code pane closed at start-handles',
+      { tag: ['@skipWin'] },
+      async ({ page, homePage }) => {
         // Load the app with the code panes
         await page.addInitScript(async (persistModelingContext) => {
           localStorage.setItem(
@@ -337,7 +344,8 @@ test.describe('Sketch tests', () => {
           )
         }, PERSIST_MODELING_CONTEXT)
         await doEditSegmentsByDraggingHandle(page, homePage, [])
-    })
+      }
+    )
   })
 
   test('Can edit a circle center and radius by dragging its handles', async ({

package.json

@@ -209,5 +209,6 @@
     "wasm-pack": "^0.13.1",
     "ws": "^8.17.0",
     "yarn": "^1.22.22"
-  }
+  },
+  "packageManager": "yarn@1.22.22+sha512.a6b2f7906b721bba3d67d4aff083df04dad64c399707841b7acf00f6b133b7ac24255f2652fa22ae3534329dc6180534e98d17432037ff6fd140556e2bb3137e"
 }

src/lang/std/engineConnection.ts

@@ -248,6 +248,8 @@ class EngineConnection extends EventTarget {
   mediaStream?: MediaStream
   idleMode: boolean = false
   promise?: Promise<void>
+  sdpAnswer?: Models['RtcSessionDescription_type']
+  triggeredStart = false
 
   onIceCandidate = function (
     this: RTCPeerConnection,
@@ -553,6 +555,7 @@ class EngineConnection extends EventTarget {
    * did not establish.
    */
   connect(reconnecting?: boolean): Promise<void> {
+    const that = this
     return new Promise((resolve) => {
       if (this.isConnecting() || this.isReady()) {
         return
@@ -583,8 +586,38 @@ class EngineConnection extends EventTarget {
           },
         }
 
+        const initiateConnectingExclusive = () => {
+          if (that.triggeredStart) return
+          that.triggeredStart = true
+
+          // Start connecting.
+          that.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.WebRTCConnecting,
+            },
+          }
+
+          // As soon as this is set, RTCPeerConnection tries to
+          // establish a connection.
+          // @ts-expect-error: Have to ignore because dom.ts doesn't have the right type
+          void that.pc?.setRemoteDescription(that.sdpAnswer)
+
+          that.state = {
+            type: EngineConnectionStateType.Connecting,
+            value: {
+              type: ConnectingType.SetRemoteDescription,
+            },
+          }
+        }
+
         this.onIceCandidate = (event: RTCPeerConnectionIceEvent) => {
+          console.log('icecandidate', event.candidate)
+
+          // This is null when the ICE gathering state is done.
+          // Windows ONLY uses this to signal it's done!
           if (event.candidate === null) {
+            initiateConnectingExclusive()
             return
           }
 
@@ -595,7 +628,6 @@ class EngineConnection extends EventTarget {
             },
           }
 
-          // Request a candidate to use
           this.send({
             type: 'trickle_ice',
             candidate: {
@@ -605,8 +637,38 @@ class EngineConnection extends EventTarget {
               usernameFragment: event.candidate.usernameFragment || undefined,
             },
           })
+
+          // Sometimes the remote end doesn't report the end of candidates.
+          // They have 3 seconds to.
+          setTimeout(() => {
+            initiateConnectingExclusive()
+          }, 3000)
         }
         this.pc?.addEventListener?.('icecandidate', this.onIceCandidate)
+        this.pc?.addEventListener?.(
+          'icegatheringstatechange',
+          function (_event) {
+            console.log('icegatheringstatechange', this.iceGatheringState)
+
+            if (this.iceGatheringState !== 'complete') return
+            initiateConnectingExclusive()
+          }
+        )
+
+        this.pc?.addEventListener?.(
+          'iceconnectionstatechange',
+          function (_event) {
+            console.log('iceconnectionstatechange', this.iceConnectionState)
+            console.log('iceconnectionstatechange', this.iceGatheringState)
+          }
+        )
+        this.pc?.addEventListener?.('negotiationneeded', function (_event) {
+          console.log('negotiationneeded', this.iceConnectionState)
+          console.log('negotiationneeded', this.iceGatheringState)
+        })
+        this.pc?.addEventListener?.('signalingstatechange', function (event) {
+          console.log('signalingstatechange', this.signalingState)
+        })
 
         this.onIceCandidateError = (_event: Event) => {
           const event = _event as RTCPeerConnectionIceErrorEvent
@@ -634,6 +696,8 @@ class EngineConnection extends EventTarget {
                 })
               )
               break
+            case 'connecting':
+              break
             case 'disconnected':
             case 'failed':
               this.pc?.removeEventListener('icecandidate', this.onIceCandidate)
@@ -1126,25 +1190,8 @@ class EngineConnection extends EventTarget {
                 },
               }
 
-              // As soon as this is set, RTCPeerConnection tries to
+              this.sdpAnswer = answer
-              // establish a connection.
-              // @ts-ignore
-              // Have to ignore because dom.ts doesn't have the right type
-              void this.pc?.setRemoteDescription(answer)
 
-              this.state = {
-                type: EngineConnectionStateType.Connecting,
-                value: {
-                  type: ConnectingType.SetRemoteDescription,
-                },
-              }
-
-              this.state = {
-                type: EngineConnectionStateType.Connecting,
-                value: {
-                  type: ConnectingType.WebRTCConnecting,
-                },
-              }
               break
 
             case 'trickle_ice':
@@ -1235,6 +1282,7 @@ class EngineConnection extends EventTarget {
     if (closedPc && closedUDC && closedWS) {
       // Do not notify the rest of the program that we have cut off anything.
       this.state = { type: EngineConnectionStateType.Disconnected }
+      this.triggeredStart = false
     }
   }
 }