fix(driver): harden timeout diagnostics suggestions

Author: lucgonp

packages/driver/src/cypress/timeout_diagnostics.ts

@@ -76,7 +76,7 @@ export class TimeoutDiagnostics {
 
     // Check for dynamic content indicators
     if (this.COMMON_PATTERNS.dynamicContent.test(selector)) {
-      const escapedSelector = selector.replace(/'/g, '\\\'')
+      const escapedSelector = this.escapeSelector(selector)
 
       suggestions.push({
         reason: 'The selector appears to target dynamic/loading content that may not be ready yet',
@@ -92,8 +92,8 @@ export class TimeoutDiagnostics {
 
     // Check for potentially incorrect selectors
     if (selector.includes(' ') && !selector.includes('[') && command === 'get') {
-      const escapedFirst = selector.split(' ')[0].replace(/'/g, '\\\'')
-      const escapedRest = selector.split(' ').slice(1).join(' ').replace(/'/g, '\\\'')
+      const escapedFirst = this.escapeSelector(selector.split(' ')[0])
+      const escapedRest = this.escapeSelector(selector.split(' ').slice(1).join(' '))
 
       suggestions.push({
         reason: 'Complex selector detected - might be fragile or incorrect',
@@ -107,18 +107,21 @@ export class TimeoutDiagnostics {
     }
 
     // Check for ID selectors that might be dynamic
-    if (selector.startsWith('#') && /\d{3,}/.test(selector)) {
-      const prefix = selector.split(/\d/)[0]
-      const escapedPrefix = prefix.replace(/'/g, '\\\'')
-
-      suggestions.push({
-        reason: 'Selector uses an ID with numbers - might be dynamically generated',
-        suggestions: [
-          'Dynamic IDs change between sessions and will cause flaky tests',
-          'Use a data-cy attribute or a more stable selector instead',
-          `If the ID is dynamic, use a partial match: cy.get('[id^="${escapedPrefix}"]').first()`,
-        ],
-      })
+    if (selector.startsWith('#')) {
+      const prefixMatch = selector.match(/^#([^\d]+)\d{3,}/)
+
+      if (prefixMatch) {
+        const escapedPrefix = this.escapeSelector(prefixMatch[1])
+
+        suggestions.push({
+          reason: 'Selector uses an ID with numbers - might be dynamically generated',
+          suggestions: [
+            'Dynamic IDs change between sessions and will cause flaky tests',
+            'Use a data-cy attribute or a more stable selector instead',
+              `If the ID is dynamic, use a partial match: cy.get('[id^="${escapedPrefix}"]').first()`,
+          ],
+        })
+      }
     }
 
     return suggestions
@@ -187,37 +190,17 @@ export class TimeoutDiagnostics {
 
   private static getGeneralSuggestions (context: TimeoutContext): DiagnosticSuggestion {
     const { command, timeout, selector } = context
-    const escapedSelector = selector?.replace(/'/g, '\\\'')
-    const actionCommands = new Set([
-      'click', 'type', 'dblclick', 'rightclick', 'check', 'uncheck',
-      'select', 'focus', 'blur', 'submit', 'trigger',
-    ])
-
-    const timeoutSuggestion = (() => {
-      if (escapedSelector && actionCommands.has(command)) {
-        return `Increase timeout if needed: cy.get('${escapedSelector}').${command}({ timeout: ${timeout * 2} })`
-      }
-
-      const args: string[] = []
-
-      if (escapedSelector) {
-        args.push(`'${escapedSelector}'`)
-      }
-
-      args.push(`{ timeout: ${timeout * 2} }`)
-
-      return `Increase timeout if needed: cy.${command}(${args.join(', ')})`
-    })()
+    const escapedSelector = selector ? this.escapeSelector(selector) : undefined
 
     const generalSuggestions = [
-      timeoutSuggestion,
+      `Increase timeout if needed: cy.${command}(${escapedSelector ? `'${escapedSelector}', ` : ''}{ timeout: ${timeout * 2} })`,
       'Verify the element/condition you\'re waiting for actually appears',
       'Check the browser console and Network tab for errors',
       'Use .debug() before the failing command to inspect the state: cy.debug()',
     ]
 
     // Add command-specific suggestions
-    if (['get', 'contains'].includes(command) && escapedSelector) {
+    if (command === 'get' && escapedSelector) {
       generalSuggestions.unshift(
         `Verify selector in DevTools: document.querySelector('${escapedSelector}')`,
         'Ensure the element is not hidden by CSS (display: none, visibility: hidden)',
@@ -238,6 +221,14 @@ export class TimeoutDiagnostics {
     }
   }
 
+  private static escapeSelector (selector: string): string {
+    return selector
+    .replace(/\\/g, '\\\\')
+    .replace(/`/g, '\\`')
+    .replace(/\$\{/g, '\\${')
+    .replace(/'/g, '\\\'')
+  }
+
   /**
    * Format diagnostic suggestions into a readable message
    */

packages/driver/test/unit/cypress/timeout_diagnostics.spec.ts

@@ -23,14 +23,14 @@ describe('TimeoutDiagnostics', () => {
     it('escapes quotes in dynamic content selector suggestions', () => {
       const context = {
         command: 'get',
-        selector: "[data-test='loading']",
+        selector: '[data-test=\'loading\']',
         timeout: 4000,
       }
 
       const suggestions = TimeoutDiagnostics.analyze(context)
 
       expect(suggestions).toHaveLength(1)
-      expect(suggestions[0].suggestions.some((s) => s.includes("\\'"))).toBe(true)
+      expect(suggestions[0].suggestions.some((s) => s.includes('\\\''))).toBe(true)
     })
 
     it('detects complex selectors', () => {
@@ -55,6 +55,9 @@ describe('TimeoutDiagnostics', () => {
       const suggestions = TimeoutDiagnostics.analyze(context)
 
       expect(suggestions.some((s) => s.reason.includes('dynamically generated'))).toBe(true)
+      const dynamicIdSuggestion = suggestions.find((s) => s.reason.includes('dynamically generated'))
+
+      expect(dynamicIdSuggestion?.suggestions.some((tip) => tip.includes('[id^="user-"'))).toBe(true)
     })
 
     it('detects network issues with many pending requests', () => {
@@ -94,9 +97,9 @@ describe('TimeoutDiagnostics', () => {
       const suggestions = TimeoutDiagnostics.analyze(context)
 
       expect(suggestions.some((s) => s.reason.includes('Animations are still running'))).toBe(true)
-      expect(suggestions.some((s) => 
-        s.suggestions.some((sug) => sug.includes('waitForAnimations: false')),
-      )).toBe(true)
+      expect(suggestions.some((s) => {
+        return s.suggestions.some((sug) => sug.includes('waitForAnimations: false'))
+      })).toBe(true)
     })
 
     it('detects excessive DOM mutations', () => {
@@ -126,6 +129,21 @@ describe('TimeoutDiagnostics', () => {
       expect(suggestions[0].suggestions.length).toBeGreaterThan(0)
     })
 
+    it('avoids document.querySelector advice for contains text queries', () => {
+      const context = {
+        command: 'contains',
+        selector: 'Success',
+        timeout: 4000,
+      }
+
+      const suggestions = TimeoutDiagnostics.analyze(context)
+
+      expect(suggestions).toHaveLength(1)
+      const combinedSuggestions = suggestions[0].suggestions.join('\n')
+
+      expect(combinedSuggestions.includes('document.querySelector')).toBe(false)
+    })
+
     it('provides command-specific suggestions for click', () => {
       const context = {
         command: 'click',
@@ -135,9 +153,9 @@ describe('TimeoutDiagnostics', () => {
 
       const suggestions = TimeoutDiagnostics.analyze(context)
 
-      expect(suggestions[0].suggestions.some((s) => 
-        s.includes('visible, enabled, and not covered'),
-      )).toBe(true)
+      expect(suggestions[0].suggestions.some((s) => {
+        return s.includes('visible, enabled, and not covered')
+      })).toBe(true)
     })
   })
 
@@ -242,15 +260,15 @@ describe('TimeoutDiagnostics', () => {
     it('escapes quotes in code suggestions to prevent syntax errors', () => {
       const context = {
         command: 'get',
-        selector: "[data-test='value']",
+        selector: '[data-test=\'value\']',
         timeout: 4000,
       }
 
       const suggestions = TimeoutDiagnostics.analyze(context)
       const formatted = TimeoutDiagnostics.formatSuggestions(suggestions)
 
       // Verify quotes are escaped in suggestions
-      expect(formatted.includes("\\'")).toBe(true)
+      expect(formatted.includes('\\\'')).toBe(true)
       // Verify no unescaped single quotes that would break JS
       expect(formatted.match(/cy\.get\('\[data-test='value'\]'\)/)).toBe(null)
     })