No more flakes and make us green again (#2771)

* No more flakes and see green

* remove extra id

* try again

* again

* again

* Fix Avatar test and make snap test more robust

* Remove extra attribute

.github/workflows/playwright.yml

@@ -90,7 +90,10 @@ jobs:
     - name: build web
       run: yarn build:local
     - name: Run ubuntu/chrome snapshots
-      run: yarn playwright test --project="Google Chrome" --update-snapshots e2e/playwright/snapshot-tests.spec.ts
+      run: |
+        yarn playwright test --project="Google Chrome" --update-snapshots e2e/playwright/snapshot-tests.spec.ts
+        # remove test-results, messes with retry logic
+        rm -r test-results
       env:
         CI: true
         token: ${{ secrets.KITTYCAD_API_TOKEN_DEV }}
@@ -121,7 +124,7 @@ jobs:
     - uses: actions/upload-artifact@v4
       if: steps.git-check.outputs.modified == 'true'
       with:
-        name: playwright-report
+        name: playwright-report-ubuntu
         path: playwright-report/
         retention-days: 30
     # if have previous run results, use them
@@ -129,16 +132,18 @@ jobs:
       if: always()
       continue-on-error: true
       with:
-        name: test-results
+        name: test-results-ubuntu
         path: test-results/
     - name: Run ubuntu/chrome flow retry failures
       id: retry
       if: always()
       run: |
-        ls -1 "test-results"
-        if [[ $(ls -1 "test-results" | wc -l) == "0" ]];
-        then echo "retried=false" >> $GITHUB_OUTPUT; exit 0;
-        else echo "retried=true" >> $GITHUB_OUTPUT;
+        if [[ -d "test-results" ]];
+        then if [[ $(ls -1 "test-results" | wc -l) != "0" ]];
+          then echo "retried=true" >> $GITHUB_OUTPUT;
+          else echo "retried=false" >> $GITHUB_OUTPUT; exit 0;
+          fi;
+        else echo "retried=false" >> $GITHUB_OUTPUT; exit 0;
         fi;
         yarn playwright test --project="Google Chrome" --last-failed e2e/playwright/flow-tests.spec.ts
       env:
@@ -153,14 +158,14 @@ jobs:
     - uses: actions/upload-artifact@v4
       if: always()
       with:
-        name: test-results
+        name: test-results-ubuntu
         path: test-results/
         retention-days: 30
         overwrite: true
     - uses: actions/upload-artifact@v4
       if: always()
       with:
-        name: playwright-report
+        name: playwright-report-ubuntu
         path: playwright-report/
         retention-days: 30
         overwrite: true
@@ -227,23 +232,25 @@ jobs:
       if: ${{ always() }}
       continue-on-error: true
       with:
-        name: test-results
+        name: test-results-macos
         path: test-results/
     - name: Run macos/safari flow retry failures
       id: retry
-      continue-on-error: true
-      if: ${{ success() }}
+      if: always()
       run: |
-        if [ -d "test-results" ];
-        then echo "retried=true" >> $GITHUB_OUTPUT;
-        else echo "retried=false" >> $GITHUB_OUTPUT;
+        if [[ -d "test-results" ]];
+        then if [[ $(ls -1 "test-results" | wc -l) != "0" ]];
+          then echo "retried=true" >> $GITHUB_OUTPUT;
+          else echo "retried=false" >> $GITHUB_OUTPUT; exit 0;
+          fi;
+        else echo "retried=false" >> $GITHUB_OUTPUT; exit 0;
         fi;
         yarn playwright test --project="webkit" --last-failed e2e/playwright/flow-tests.spec.ts
       env:
         CI: true
         token: ${{ secrets.KITTYCAD_API_TOKEN_DEV }}
     - name: Run macos/safari flow
-      if: ${{ steps.retry.outputs.retried != 'true' }}
+      if: steps.retry.outputs.retried == 'false'
       # webkit doesn't work on Ubuntu because of the same reason tauri doesn't (webRTC issues)
       # TODO remove this and the matrix and run all tests on ubuntu when this is fixed
       run: yarn playwright test --project="webkit" e2e/playwright/flow-tests.spec.ts
@@ -253,14 +260,14 @@ jobs:
     - uses: actions/upload-artifact@v4
       if: ${{ always() }}
       with:
-        name: test-results
+        name: test-results-macos
         path: test-results/
         retention-days: 30
         overwrite: true
     - uses: actions/upload-artifact@v4
       if: ${{ always() }}
       with:
-        name: playwright-report
+        name: playwright-report-macos
         path: playwright-report/
         retention-days: 30
         overwrite: true

e2e/playwright/flow-tests.spec.ts

@@ -1646,8 +1646,10 @@ test.describe('Onboarding tests', () => {
     await page.waitForURL('**/file/**', { waitUntil: 'domcontentloaded' })
 
     // Test that the text in this step is correct
-    const avatarLocator = page.getByTestId('user-sidebar-toggle').locator('img')
-    const onboardingOverlayLocator = page
+    const avatarLocator = await page
+      .getByTestId('user-sidebar-toggle')
+      .locator('img')
+    const onboardingOverlayLocator = await page
       .getByTestId('onboarding-content')
       .locator('div')
       .nth(1)
@@ -1657,6 +1659,18 @@ test.describe('Onboarding tests', () => {
     await expect(onboardingOverlayLocator).toBeVisible()
     await expect(onboardingOverlayLocator).toContainText('your avatar')
 
+    // This is to force the avatar to 404.
+    // For our test image (only triggers locally. on CI, it's Kurt's /
+    // gravatar image )
+    await page.route('/cat.jpg', async (route) => {
+      await route.fulfill({
+        status: 404,
+        contentType: 'text/plain',
+        body: 'Not Found!',
+      })
+    })
+
+    // 404 the CI avatar image
     await page.route('https://lh3.googleusercontent.com/**', async (route) => {
       await route.fulfill({
         status: 404,
@@ -3098,8 +3112,11 @@ const doSnapAtDifferentScales = async (
   await u.updateCamPosition(camPos)
   await u.closeDebugPanel()
 
+  await page.mouse.move(0, 0)
+
   // select a plane
-  await page.mouse.click(700, 200)
+  await page.mouse.move(700, 200, { steps: 10 })
+  await page.mouse.click(700, 200, { delay: 200 })
   await expect(page.locator('.cm-content')).toHaveText(
     `const sketch001 = startSketchOn('-XZ')`
   )
@@ -3112,26 +3129,29 @@ const doSnapAtDifferentScales = async (
 
   // draw three lines
   await page.waitForTimeout(500)
-  await page.mouse.click(pointA[0], pointA[1])
+  await page.mouse.move(pointA[0], pointA[1], { steps: 10 })
+  await page.mouse.click(pointA[0], pointA[1], { delay: 200 })
   await page.waitForTimeout(100)
   await expect(page.locator('.cm-content')).not.toHaveText(prevContent)
   prevContent = await page.locator('.cm-content').innerText()
 
-  await page.mouse.click(pointB[0], pointB[1])
+  await page.mouse.move(pointB[0], pointB[1], { steps: 10 })
+  await page.mouse.click(pointB[0], pointB[1], { delay: 200 })
   await page.waitForTimeout(100)
   await expect(page.locator('.cm-content')).not.toHaveText(prevContent)
   prevContent = await page.locator('.cm-content').innerText()
 
-  await page.mouse.click(pointC[0], pointC[1])
+  await page.mouse.move(pointC[0], pointC[1], { steps: 10 })
+  await page.mouse.click(pointC[0], pointC[1], { delay: 200 })
   await page.waitForTimeout(100)
   await expect(page.locator('.cm-content')).not.toHaveText(prevContent)
   prevContent = await page.locator('.cm-content').innerText()
 
-  await page.mouse.move(pointA[0] - 12, pointA[1] + 12)
+  await page.mouse.move(pointA[0] - 12, pointA[1] + 12, { steps: 10 })
   const pointNotQuiteA = [pointA[0] - 7, pointA[1] + 7]
   await page.mouse.move(pointNotQuiteA[0], pointNotQuiteA[1], { steps: 10 })
 
-  await page.mouse.click(pointNotQuiteA[0], pointNotQuiteA[1])
+  await page.mouse.click(pointNotQuiteA[0], pointNotQuiteA[1], { delay: 200 })
   await expect(page.locator('.cm-content')).not.toHaveText(prevContent)
   prevContent = await page.locator('.cm-content').innerText()
 

playwright.config.ts

@@ -12,13 +12,13 @@ import { defineConfig, devices } from '@playwright/test'
 export default defineConfig({
   testDir: './e2e/playwright',
   /* Run tests in files in parallel */
-  fullyParallel: false,
+  fullyParallel: true,
   /* Fail the build on CI if you accidentally left test.only in the source code. */
   forbidOnly: !!process.env.CI,
   /* Retry on CI only */
   retries: process.env.CI ? 3 : 0,
   /* Different amount of parallelism on CI and local. */
-  workers: process.env.CI ? 1 : 4,
+  workers: process.env.CI ? 4 : 4,
   /* Reporter to use. See https://playwright.dev/docs/test-reporters */
   reporter: 'html',
   /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */

src/components/Stream.tsx

@@ -127,8 +127,8 @@ export const Stream = ({ className = '' }: { className?: string }) => {
   return (
     <div
       className="absolute inset-0 z-0"
-      data-testid="stream"
       id="stream"
+      data-testid="stream"
       onMouseUp={handleMouseUp}
       onMouseDown={handleMouseDown}
       onContextMenu={(e) => e.preventDefault()}

src/components/UserSidebarMenu.tsx

@@ -20,6 +20,12 @@ const UserSidebarMenu = ({ user }: { user?: User }) => {
   const navigate = useNavigate()
   const send = useSettingsAuthContext()?.auth?.send
 
+  // This image host goes down sometimes. We will instead rewrite the
+  // resource to be a local one.
+  if (user?.image === 'https://placekitten.com/200/200') {
+    user.image = '/cat.jpg'
+  }
+
   // Fallback logic for displaying user's "name":
   // 1. user.name
   // 2. user.first_name + ' ' + user.last_name

src/routes/Onboarding/UserMenu.tsx

@@ -19,8 +19,12 @@ export default function UserMenu() {
       '[data-testid="user-sidebar-toggle"] img'
     )
 
+    const onError = () => setAvatarErrored(true)
     if (element?.tagName === 'IMG') {
-      element.addEventListener('error', () => setAvatarErrored(true))
+      element?.addEventListener('error', onError)
+    }
+    return () => {
+      element?.removeEventListener('error', onError)
     }
   }, [])