ci: Move flaky npm tests into specific scheduled API Control script (#1154)

* Moving failing tests into their own directory

* ci: Add API Control daily checks

* chore: cleanup

Author: pixelastic

.github/workflows/api-control.yml

@@ -0,0 +1,25 @@
+name: API CONTROL
+on:
+  schedule:
+    # Everyday, at 10am
+    - cron: '0 10 * * *'
+
+jobs:
+  api-control:
+    name: 🛂 API Control
+    runs-on: ubuntu-latest
+    steps:
+      - name: 📁 Checkout code
+        uses: actions/checkout@v4
+
+      - name: ⚙️ Setup node
+        uses: actions/setup-node@v3
+        with:
+          node-version: '18.18'
+          cache: 'yarn'
+
+      - name: 📦 Install dependencies
+        run: yarn install --frozen-lockfile
+
+      - name: 🛂 API Control
+        run: GITHUB_RUN_ID="${{ github.run_id }}" yarn test:api-control

jest.config.js

@@ -1,11 +1,12 @@
 // eslint-disable-next-line import/no-commonjs
 module.exports = {
-  name: 'npm',
-
   transform: {
     '^.+\\.[jt]sx?$': 'ts-jest',
   },
   testMatch: ['<rootDir>/src/**/*.test.[jt]s'],
+  // By default, ignore the slow and flaky tests testing external APIs. Those
+  // will be run specifically with `yarn run test:api-control`
+  testPathIgnorePatterns: ['api-control'],
   globals: {
     'ts-jest': {
       diagnostics: false,

package.json

@@ -16,6 +16,7 @@
     "start": "UV_THREADPOOL_SIZE=64 node --max-old-space-size=1500 dist/index.js",
     "start_new": "indexName=npm-search-new bootstrapIndexName=npm-search-new.tmp UV_THREADPOOL_SIZE=64 node --max-old-space-size=1500 dist/index.js",
     "test:watch": "jest --watchAll --no-watchman",
+    "test:api-control": "./scripts/test-api-control",
     "test": "jest --forceExit",
     "publish:check": "node ./scripts/publish-check.js",
     "publish:github": "./scripts/publish-github",

scripts/test-api-control

@@ -0,0 +1,50 @@
+#!/bin/sh
+# API Control tests assert that the external third party APIs we are using
+# return data in an expected format. They query the relevant API with real (not
+# mocked) HTTP calls.
+#
+# As those tests are slow, and have a higher probability of flakiness because of
+# network issues or timeouts, we don't want to run them on each pre-commit hook
+# or CI commit. They can instead be run manually, or periodically from the CI.
+#
+# When it fails on the CI, it will generate a GitHub issue with the failure details
+# as well as a link to the run.
+
+# Running locally, with colors and live output
+if [ "$GITHUB_RUN_ID" = "" ]; then
+	jest \
+		./src/__tests__/api-control \
+		--forceExit \
+		--testPathIgnorePatterns=''
+	exit $?
+fi
+
+# Running on CI, creating an issue on failure
+echo "Wait while we run the tests"
+output=$(jest \
+	./src/__tests__/api-control \
+	--forceExit \
+	--testPathIgnorePatterns='' 2>&1)
+exitCode=$?
+echo "$output"
+
+# Stop on success
+if [ "$exitCode" = "0" ]; then
+	exit 0
+fi
+
+# Create the issue on failure
+gh issue create \
+	--title "API Control failed" \
+	--body "\
+One of the external APIs we depend on failed to return coherent data in our periodic test.
+Maybe it's a temporary issue, maybe they changed their format.
+
+https://github.com/algolia/npm-search/actions/runs/$GITHUB_RUN_ID
+
+\`\`\`
+$output
+\`\`\`"
+
+# Still mark the job as failed
+exit 1

src/__tests__/api-control/npm.test.ts

@@ -0,0 +1,54 @@
+import * as api from '../../npm/index';
+
+jest.setTimeout(15000);
+
+describe('findAll()', () => {
+  it('contains the correct keys', async () => {
+    const all = await api.findAll({ limit: 2, startkey: '0' });
+
+    expect(all).toEqual(
+      expect.objectContaining({
+        offset: expect.any(Number),
+        total_rows: expect.any(Number),
+      })
+    );
+
+    expect(all.rows).toHaveLength(2);
+
+    expect(all.rows[0]).toEqual(
+      expect.objectContaining({
+        id: '0',
+        key: '0',
+        value: { rev: '11-61bb2c49ce3202a3e0ab9a65646b4b4d' },
+      })
+    );
+  });
+});
+
+describe('getDoc()', () => {
+  it('retrieves a single doc', async () => {
+    const doc = await api.getDoc(
+      'jsdelivr',
+      '8-734f30eea3baad0a62452a3bff1dd116'
+    );
+
+    expect(doc.name).toBe('jsdelivr');
+    expect(Object.keys(doc.versions)).toHaveLength(2);
+  });
+});
+
+describe('getInfo()', () => {
+  let registryInfo;
+  beforeAll(async () => {
+    registryInfo = await api.getInfo();
+  });
+
+  it('contains the correct keys', () => {
+    expect(registryInfo).toEqual(
+      expect.objectContaining({
+        nbDocs: expect.any(Number),
+        seq: expect.any(Number),
+      })
+    );
+  });
+});

src/npm/__tests__/index.test.ts

@@ -5,41 +5,6 @@ import { computeDownload } from '../index';
 
 jest.setTimeout(15000);
 
-describe('findAll()', () => {
-  it('contains the correct keys', async () => {
-    const all = await api.findAll({ limit: 2, startkey: '0' });
-
-    expect(all).toEqual(
-      expect.objectContaining({
-        offset: expect.any(Number),
-        total_rows: expect.any(Number),
-      })
-    );
-
-    expect(all.rows).toHaveLength(2);
-
-    expect(all.rows[0]).toEqual(
-      expect.objectContaining({
-        id: '0',
-        key: '0',
-        value: { rev: '11-61bb2c49ce3202a3e0ab9a65646b4b4d' },
-      })
-    );
-  });
-});
-
-describe('getDoc()', () => {
-  it('retrieves a single doc', async () => {
-    const doc = await api.getDoc(
-      'jsdelivr',
-      '8-734f30eea3baad0a62452a3bff1dd116'
-    );
-
-    expect(doc.name).toBe('jsdelivr');
-    expect(Object.keys(doc.versions)).toHaveLength(2);
-  });
-});
-
 describe('getDocFromRegistry()', () => {
   it('retrieves a single doc', async () => {
     const doc = await api.getDocFromRegistry('jsdelivr');
@@ -61,22 +26,6 @@ describe('getDocFromRegistry()', () => {
   });
 });
 
-describe('getInfo()', () => {
-  let registryInfo;
-  beforeAll(async () => {
-    registryInfo = await api.getInfo();
-  });
-
-  it('contains the correct keys', () => {
-    expect(registryInfo).toEqual(
-      expect.objectContaining({
-        nbDocs: expect.any(Number),
-        seq: expect.any(Number),
-      })
-    );
-  });
-});
-
 describe('getDependents()', () => {
   let dependents;
   beforeAll(async () => {