use assertWithRetries in deletion tests to ease flake

pinecone-io · Jan 12, 2024 · 561813c · 561813c
1 parent 4fa1207
commit 561813c
Show file tree

Hide file tree

Showing 2 changed files with 80 additions and 63 deletions.
diff --git a/.github/workflows/testing.yml b/.github/workflows/testing.yml
@@ -81,6 +81,10 @@ jobs:
         with:
           node-version: 18.x
           registry-url: 'https://registry.npmjs.org'
+      - name: Install npm packages
+        run: |
+          npm install --ignore-scripts
+        shell: bash
       - name: Run integration cleanup command
         env:
           CI: true

diff --git a/src/integration/data/delete.test.ts b/src/integration/data/delete.test.ts
@@ -1,9 +1,9 @@
 import { Pinecone, Index } from '../../index';
 import {
+  assertWithRetries,
   randomString,
   generateRecords,
   INDEX_NAME,
-  sleep,
   waitUntilRecordsReady,
 } from '../test-helpers';
 
@@ -49,39 +49,39 @@ describe('delete', () => {
     await ns.upsert(recordToUpsert);
 
     // Await record freshness, and check record upserted
-    let stats = await waitUntilRecordsReady(ns, namespace, recordIds);
+    const stats = await waitUntilRecordsReady(ns, namespace, recordIds);
     if (stats.namespaces) {
       expect(stats.namespaces[namespace].recordCount).toEqual(1);
     } else {
       fail('Expected namespaces to be defined');
     }
 
     // Look more closely at one of the records to make sure values set
-    const fetchResult = await ns.fetch(['0']);
-    const records = fetchResult.records;
-    if (records) {
-      expect(records['0'].id).toEqual('0');
-      expect(records['0'].values.length).toEqual(5);
-    } else {
-      fail(
-        'Did not find expected records. Fetch result was ' +
-          JSON.stringify(fetchResult)
-      );
-    }
+    const fetchAssertions = [
+      (results) => {
+        if (results.records) {
+          expect(results.records['0'].id).toEqual('0');
+          expect(results.records['0'].values.length).toEqual(5);
+        } else {
+          fail(
+            'Did not find expected records. Fetch result was ' +
+              JSON.stringify(results)
+          );
+        }
+      },
+    ];
+    assertWithRetries(() => ns.fetch(['0']), fetchAssertions);
 
     // Try deleting the record
     await ns.deleteOne('0');
-    await sleep(3000);
 
     // Verify the record is removed
-    stats = await ns.describeIndexStats();
-    if (stats.namespaces) {
-      expect(stats.namespaces[namespace]).toBeUndefined();
-    } else {
-      // no-op. This shouldn't actually happen unless there
-      // are leftover namespaces from previous runs that
-      // failed or stopped without proper cleanup.
-    }
+    const deleteAssertions = [
+      (stats) => {
+        expect(stats.namespaces[namespace]).toBeUndefined();
+      },
+    ];
+    assertWithRetries(() => ns.describeIndexStats(), deleteAssertions);
   });
 
   test('verify deleteMany with ids', async () => {
@@ -95,63 +95,76 @@ describe('delete', () => {
     await ns.upsert(recordsToUpsert);
 
     // Await record freshness, and check records upserted
-    let stats = await waitUntilRecordsReady(ns, namespace, recordIds);
+    const stats = await waitUntilRecordsReady(ns, namespace, recordIds);
     if (stats.namespaces) {
       expect(stats.namespaces[namespace].recordCount).toEqual(3);
     } else {
       fail('Expected namespaces to be defined');
     }
 
     // Look more closely at one of the records to make sure values set
-    const fetchResult = await ns.fetch(['0']);
-    const records = fetchResult.records;
-    if (records) {
-      expect(records['0'].id).toEqual('0');
-      expect(records['0'].values.length).toEqual(5);
-    } else {
-      fail(
-        'Did not find expected records. Fetch result was ' +
-          JSON.stringify(fetchResult)
-      );
-    }
+    const fetchAssertions = [
+      (results) => {
+        if (results.records) {
+          expect(results.records['0'].id).toEqual('0');
+          expect(results.records['0'].values.length).toEqual(5);
+        } else {
+          fail(
+            'Did not find expected records. Fetch result was ' +
+              JSON.stringify(results)
+          );
+        }
+      },
+    ];
+    assertWithRetries(() => ns.fetch(['0']), fetchAssertions);
 
     // Try deleting 2 of 3 records
     await ns.deleteMany(['0', '2']);
-    await sleep(3000);
-    stats = await ns.describeIndexStats();
-    if (stats.namespaces) {
-      expect(stats.namespaces[namespace].recordCount).toEqual(1);
-    } else {
-      fail(
-        'Expected namespaces to be defined (second call). Stats were ' +
-          JSON.stringify(stats)
-      );
-    }
+
+    const deleteAssertions = [
+      (stats) => {
+        if (stats.namespaces) {
+          expect(stats.namespaces[namespace].recordCount).toEqual(1);
+        } else {
+          fail(
+            'Expected namespaces to be defined (second call). Stats were ' +
+              JSON.stringify(stats)
+          );
+        }
+      },
+    ];
+    assertWithRetries(() => ns.describeIndexStats(), deleteAssertions);
 
     // Check that record id='1' still exists
-    const fetchResult2 = await ns.fetch(['1']);
-    const records2 = fetchResult2.records;
-    if (records2) {
-      expect(records2['1']).not.toBeUndefined();
-    } else {
-      fail(
-        'Expected record 2 to be defined. Fetch result was ' +
-          JSON.stringify(fetchResult2)
-      );
-    }
+    const fetchAssertions2 = [
+      (results) => {
+        if (results.records2) {
+          expect(results.records2['1']).not.toBeUndefined();
+        } else {
+          fail(
+            'Expected record 2 to be defined. Fetch result was ' +
+              JSON.stringify(results)
+          );
+        }
+      },
+    ];
+    assertWithRetries(() => ns.fetch(['1']), fetchAssertions2);
 
     // deleting non-existent records should not throw
     await ns.deleteMany(['0', '1', '2', '3']);
-    await sleep(3000);
 
     // Verify all are now removed
-    stats = await ns.describeIndexStats();
-    if (stats.namespaces) {
-      expect(stats.namespaces[namespace]).toBeUndefined();
-    } else {
-      // no-op. This shouldn't actually happen unless there
-      // are leftover namespaces from previous runs that
-      // failed or stopped without proper cleanup.
-    }
+    const deleteAssertions2 = [
+      (stats) => {
+        if (stats.namespaces) {
+          expect(stats.namespaces[namespace]).toBeUndefined();
+        } else {
+          // no-op. This shouldn't actually happen unless there
+          // are leftover namespaces from previous runs that
+          // failed or stopped without proper cleanup.
+        }
+      },
+    ];
+    assertWithRetries(() => ns.describeIndexStats(), deleteAssertions2);
   });
 });