fix: use dumb wait instead of diagnostic watch

ast-grep · Feb 10, 2024 · c2a51fc · c2a51fc
1 parent 0aafd6a
commit c2a51fc
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 92 deletions.
diff --git a/src/test/fileModifications.test.ts b/src/test/fileModifications.test.ts
@@ -1,17 +1,15 @@
 import * as vscode from 'vscode'
 import * as yaml from 'js-yaml'
 
-import {
-  getDocUri,
-  waitForDiagnosticChange,
-  assertDiagnosticsEqual
-} from './utils'
-
-const MAX_WAIT_TIME_FOR_UPDATE = 30000 // ms
-const MAX_WAIT_TIME_FOR_INITIAL_DIAGNOSTICS = 10000 // ms
+import { getDocUri, sleep, assertDiagnosticsEqual } from './utils'
 
 const docUri = getDocUri('test.ts')
-const diagnosticss = getExpectedDiagnosticss()
+const diagnostics = getExpectedDiagnosticss()
+
+// Code cannot guarantee file change event is propagated to host system.
+// Thus watching new diagnostics is not reliable.
+// We should look at eventual consistency. Just Wait.
+const WAIT_FOR_FS_CHANGE = 300
 
 async function writeNewRule() {
   let vscodeuri = vscode.Uri.file(
@@ -58,57 +56,39 @@ suite('Should update when files change', () => {
    * Locally, this test could probably be simplified to one line, but in the CI environment, the initial diagnostics may not be ready immediately.
    */
   test('Ensure initial diagnostics are correct', async () => {
-    try {
-      assertDiagnosticsEqual(
-        vscode.languages.getDiagnostics(docUri),
-        diagnosticss[0]
-      )
-    } catch (e) {
-      console.warn(
-        "We are testing how the extension reacts to file modifications, but the initial diagnostics are not as expected. Let's wait briefly for them to update."
-      )
-      console.warn(e)
-      try {
-        await waitForDiagnosticChange(MAX_WAIT_TIME_FOR_INITIAL_DIAGNOSTICS)
-      } catch (timeout) {
-        console.warn(
-          'Diagnostics did not update within the expected time frame.'
-        )
-        // Even though waitForDiagnosticChange timed out, let's continue and see if the diagnostics are correct now.
-      }
-      assertDiagnosticsEqual(
-        vscode.languages.getDiagnostics(docUri),
-        diagnosticss[0]
-      )
-    }
+    await sleep(WAIT_FOR_FS_CHANGE)
+    assertDiagnosticsEqual(
+      vscode.languages.getDiagnostics(docUri),
+      diagnostics[0]
+    )
   })
   test('Update on new rule creation', async () => {
     writeNewRule()
-    await waitForDiagnosticChange(MAX_WAIT_TIME_FOR_UPDATE)
+    await sleep(WAIT_FOR_FS_CHANGE)
     assertDiagnosticsEqual(
       vscode.languages.getDiagnostics(docUri),
-      diagnosticss[1]
+      diagnostics[1]
     )
   })
   test('Update on rule deletion', async () => {
     deleteNewRule()
-    await waitForDiagnosticChange(MAX_WAIT_TIME_FOR_UPDATE)
+    await sleep(WAIT_FOR_FS_CHANGE)
     assertDiagnosticsEqual(
       vscode.languages.getDiagnostics(docUri),
-      diagnosticss[2]
+      diagnostics[2]
     )
   })
   test('Update on ruleDirs change to nonexistent path', async () => {
     await setRuleDirs(['NoRules'])
-    await waitForDiagnosticChange(MAX_WAIT_TIME_FOR_UPDATE)
+    await sleep(WAIT_FOR_FS_CHANGE)
     assertDiagnosticsEqual(vscode.languages.getDiagnostics(docUri), [])
   })
   test('Update on ruleDirs change back to real path', async () => {
     await setRuleDirs(['rules'])
-    await waitForDiagnosticChange(MAX_WAIT_TIME_FOR_UPDATE)
+    await sleep(WAIT_FOR_FS_CHANGE)
     assertDiagnosticsEqual(
       vscode.languages.getDiagnostics(docUri),
-      diagnosticss[0]
+      diagnostics[0]
     )
   })
 })

diff --git a/src/test/utils.ts b/src/test/utils.ts
@@ -58,62 +58,10 @@ export const assertDiagnosticsEqual = (
   })
 }
 
-/**
- * Generic function that takes a callback-based function and converts it to a Promise-based function
- * It would also be reasonable to import this from a library like util.promisify
- * @param func The callback-based function to be converted (think 'setTimeout')
- * @param optionalTimeout The optional timeout in milliseconds to wait for the callback to be called
- * @param errorText The optional error text to throw if the timeout is reached
- */
-const promisify =
-  (func: Function, optionalTimeout?: number, errorText: string = 'Timeout') =>
-  (...args: any[]) => {
-    const mainPromise = new Promise((resolve, _) =>
-      func(...args, (result: any) => resolve(result))
-    )
-    if (optionalTimeout !== undefined) {
-      return Promise.race([
-        mainPromise,
-        new Promise((_, reject) =>
-          setTimeout(() => reject(new Error(errorText)), optionalTimeout)
-        )
-      ])
-    } else {
-      return mainPromise
-    }
-  }
-
-/**
- * Helper function allowing us to wait for a vscode.DiagnosticChangeEvent to be fired
- * This is useful for tests where we modify a file and then want to wait for the diagnostics to be updated
- * @param optionalTimeout The optional timeout in milliseconds to wait for the callback to be called. If no timeout is provided, the function will wait indefinitely.
- *
- */
-export const waitForDiagnosticChange = async (optionalTimeout?: number) => {
-  let disposable: vscode.Disposable | undefined
-  try {
-    return await promisify(
-      (handler: (e: vscode.DiagnosticChangeEvent) => vscode.Disposable) => {
-        disposable = vscode.languages.onDidChangeDiagnostics(handler)
-        return disposable
-      },
-      optionalTimeout,
-      'Took too long waiting for diagnostics to change. Limit was set at ' +
-        optionalTimeout +
-        'ms'
-    )()
-  } finally {
-    if (disposable) {
-      disposable.dispose()
-    }
-  }
-}
-
 /**
  * @param ms The number of milliseconds to sleep
- * @returns
  */
-async function sleep(ms: number) {
+export async function sleep(ms: number): Promise<void> {
   return new Promise(resolve => setTimeout(resolve, ms))
 }