feat: Skip OpenAPI on test failure

.github/workflows/main.yml

@@ -27,7 +27,7 @@ jobs:
         run: yarn install
 
       - name: Install AppMap tools
-        uses: getappmap/install-action@v1.0
+        uses: getappmap/install-action@main
         with:
           tools-url: https://github.com/getappmap/appmap-js/releases/download/%40appland%2Fappmap-faster-v1.0-pre.2/appmap-linux-x64
           github-token: ${{ secrets.GITHUB_TOKEN }}
@@ -44,7 +44,7 @@ jobs:
         uses: ./
 
       - name: Analyze AppMaps
-        uses: getappmap/analyze-action@v1-pre.16
+        uses: getappmap/analyze-action@main
         if: (success() || failure()) && github.event_name == 'pull_request'
         with:
           base-revision: ${{ github.event.pull_request.base.sha }}

dist/merge/index.js

@@ -72741,6 +72741,43 @@ class LocalCacheStore {
 exports["default"] = LocalCacheStore;
 
 
+/***/ }),
+
+/***/ 2620:
+/***/ (function(__unused_webpack_module, exports, __nccwpck_require__) {
+
+"use strict";
+
+var __awaiter = (this && this.__awaiter) || function (thisArg, _arguments, P, generator) {
+    function adopt(value) { return value instanceof P ? value : new P(function (resolve) { resolve(value); }); }
+    return new (P || (P = Promise))(function (resolve, reject) {
+        function fulfilled(value) { try { step(generator.next(value)); } catch (e) { reject(e); } }
+        function rejected(value) { try { step(generator["throw"](value)); } catch (e) { reject(e); } }
+        function step(result) { result.done ? resolve(result.value) : adopt(result.value).then(fulfilled, rejected); }
+        step((generator = generator.apply(thisArg, _arguments || [])).next());
+    });
+};
+Object.defineProperty(exports, "__esModule", ({ value: true }));
+const promises_1 = __nccwpck_require__(3292);
+const glob_1 = __nccwpck_require__(5029);
+const path_1 = __nccwpck_require__(1017);
+function anyTestFailed(appmapDir) {
+    return __awaiter(this, void 0, void 0, function* () {
+        let testFailed = false;
+        const metadataFiles = yield (0, glob_1.glob)((0, path_1.join)(appmapDir, '**', 'metadata.json'));
+        for (const metadataFile of metadataFiles) {
+            const metadata = (yield (0, promises_1.readFile)(metadataFile, 'utf-8'));
+            if ((metadata === null || metadata === void 0 ? void 0 : metadata.test_status) === 'failed') {
+                testFailed = true;
+                break;
+            }
+        }
+        return testFailed;
+    });
+}
+exports["default"] = anyTestFailed;
+
+
 /***/ }),
 
 /***/ 8659:
@@ -73009,6 +73046,7 @@ const CLIArchiveCommand_1 = __importDefault(__nccwpck_require__(8634));
 const LocalArtifactStore_1 = __importDefault(__nccwpck_require__(3656));
 const LocalCacheStore_1 = __importDefault(__nccwpck_require__(5419));
 const setVerbose_1 = __nccwpck_require__(5284);
+const anyTestFailed_1 = __importDefault(__nccwpck_require__(2620));
 class Merge extends ArchiveAction_1.default {
     constructor(archiveCount) {
         super();
@@ -73067,11 +73105,16 @@ class Merge extends ArchiveAction_1.default {
                     yield (0, promises_1.cp)(file, (0, path_1.join)(appmapDir, (0, path_1.basename)(file)), { recursive: true });
                 }
             }
-            // TODO: Each archive directory already contains an openapi.yml file, so it would be
-            // quite possible, and much more efficient, to merge those files instead of generating
-            // a new one from scratch.
-            (0, log_1.default)(log_1.LogLevel.Info, 'Generating OpenAPI definitions');
-            yield this.archiveCommand.generateOpenAPI(appmapDir);
+            if (yield (0, anyTestFailed_1.default)(appmapDir)) {
+                (0, log_1.default)(log_1.LogLevel.Info, 'Skipping OpenAPI generation because at least one test failed');
+            }
+            else {
+                // TODO: Each archive directory already contains an openapi.yml file, so it would be
+                // quite possible, and much more efficient, to merge those files instead of generating
+                // a new one from scratch.
+                (0, log_1.default)(log_1.LogLevel.Info, 'Generating OpenAPI definitions');
+                yield this.archiveCommand.generateOpenAPI(appmapDir);
+            }
             (0, log_1.default)(log_1.LogLevel.Info, 'Building merged archive');
             const archiveOptions = { index: false };
             if (this.revision)

jest.config.js

@@ -3,4 +3,7 @@ module.exports = {
   preset: 'ts-jest',
   testEnvironment: 'node',
   testTimeout: parseInt(process.env.TEST_TIMEOUT, 10) || 60000, // Smoke test takes a long time
+  // There are test cases that change the process working directory, and that does
+  // not work with multiple workers.
+  maxWorkers: 1,
 };

package.json

@@ -6,6 +6,7 @@
   "main": "build/index.js",
   "scripts": {
     "compile": "tsc",
+    "watch": "tsc --watch",
     "package": "ncc build -o dist/archive --source-map src/archive.ts && ncc build -o dist/merge --source-map src/merge.ts",
     "build": "yarn compile && yarn package",
     "test": "find test -name '*.spec.ts' -exec yarn appmap-agent-js -- jest --no-cache {} \\;",

src/anyTestFailed.ts

@@ -0,0 +1,16 @@
+import {readFile} from 'fs/promises';
+import {glob} from 'glob';
+import {join} from 'path';
+
+export default async function anyTestFailed(appmapDir: string): Promise<boolean> {
+  let testFailed = false;
+  const metadataFiles = await glob(join(appmapDir, '**', 'metadata.json'));
+  for (const metadataFile of metadataFiles) {
+    const metadata = (await readFile(metadataFile, 'utf-8')) as any;
+    if (metadata?.test_status === 'failed') {
+      testFailed = true;
+      break;
+    }
+  }
+  return testFailed;
+}

src/merge.ts

@@ -10,11 +10,11 @@ import {glob} from 'glob';
 import {basename, join} from 'path';
 import {existsSync} from 'fs';
 import {ArgumentParser} from 'argparse';
-import verbose from './verbose';
 import CLIArchiveCommand from './CLIArchiveCommand';
 import LocalArtifactStore from './LocalArtifactStore';
 import LocalCacheStore from './LocalCacheStore';
 import {setVerbose} from './setVerbose';
+import anyTestFailed from './anyTestFailed';
 
 export class Merge extends ArchiveAction {
   constructor(public archiveCount: number) {
@@ -90,11 +90,15 @@ export class Merge extends ArchiveAction {
       }
     }
 
-    // TODO: Each archive directory already contains an openapi.yml file, so it would be
-    // quite possible, and much more efficient, to merge those files instead of generating
-    // a new one from scratch.
-    log(LogLevel.Info, 'Generating OpenAPI definitions');
-    await this.archiveCommand.generateOpenAPI(appmapDir);
+    if (await anyTestFailed(appmapDir)) {
+      log(LogLevel.Info, 'Skipping OpenAPI generation because at least one test failed');
+    } else {
+      // TODO: Each archive directory already contains an openapi.yml file, so it would be
+      // quite possible, and much more efficient, to merge those files instead of generating
+      // a new one from scratch.
+      log(LogLevel.Info, 'Generating OpenAPI definitions');
+      await this.archiveCommand.generateOpenAPI(appmapDir);
+    }
 
     log(LogLevel.Info, 'Building merged archive');
     const archiveOptions: ArchiveOptions = {index: false};

test/merge.spec.ts

@@ -7,12 +7,39 @@ import * as test from './helper';
 import * as locateArchiveFile from '../src/locateArchiveFile';
 import {RestoreOptions} from '../src/ArchiveCommand';
 import {executeCommand} from '../src/executeCommand';
+import * as anyTestFailed from '../src/anyTestFailed';
 
 describe('merge', () => {
   let context: test.ArchiveTestContext;
   let archiveCount = 2;
   let action: Merge;
 
+  const placeTarFile = async (revision: string) => {
+    await mkdir(join('.appmap/archive/full'), {recursive: true});
+    await writeFile(join('.appmap/archive/full', `${revision}.tar`), '# dummy file');
+  };
+
+  const unpackTarFile = async (revision: string) => {
+    await mkdir(join('.appmap/work', revision), {
+      recursive: true,
+    });
+    await executeCommand(
+      `cp -r ${join(test.FixtureDir, 'tmp/appmap')} ${join('.appmap/work', revision)}`
+    );
+    await writeFile(
+      join('.appmap/work', revision.toString(), 'appmap_archive.json'),
+      JSON.stringify(
+        {
+          config: {
+            appmap_dir: 'tmp/appmap',
+          },
+        },
+        null,
+        2
+      )
+    );
+  };
+
   beforeEach(async () => {
     context = new test.ArchiveTestContext();
     await context.setup();
@@ -25,39 +52,6 @@ describe('merge', () => {
 
     await rm(join('tmp/appmap'), {recursive: true});
     await mkdir(join('tmp/appmap'), {recursive: true});
-  });
-
-  afterEach(async () => {
-    assert(context);
-    await context.teardown();
-  });
-
-  it('fetches archives from the cache and merges them', async () => {
-    const placeTarFile = async (revision: string) => {
-      await mkdir(join('.appmap/archive/full'), {recursive: true});
-      await writeFile(join('.appmap/archive/full', `${revision}.tar`), '# dummy file');
-    };
-
-    const unpackTarFile = async (revision: string) => {
-      await mkdir(join('.appmap/work', revision), {
-        recursive: true,
-      });
-      await executeCommand(
-        `cp -r ${join(test.FixtureDir, 'tmp/appmap')} ${join('.appmap/work', revision)}`
-      );
-      await writeFile(
-        join('.appmap/work', revision.toString(), 'appmap_archive.json'),
-        JSON.stringify(
-          {
-            config: {
-              appmap_dir: 'tmp/appmap',
-            },
-          },
-          null,
-          2
-        )
-      );
-    };
 
     context.noCacheStore.restore = jest
       .fn()
@@ -80,6 +74,15 @@ describe('merge', () => {
       });
 
     jest.spyOn(locateArchiveFile, 'default').mockResolvedValue('.appmap/archive/full/402dec8.tar');
+  });
+
+  afterEach(async () => {
+    assert(context);
+    await context.teardown();
+  });
+
+  it('fetches archives from the cache and merges them', async () => {
+    jest.spyOn(anyTestFailed, 'default').mockResolvedValue(false);
 
     await action.merge();
 
@@ -101,4 +104,19 @@ describe('merge', () => {
       },
     ]);
   });
+
+  describe('when a test has failed', () => {
+    it('skips OpenAPI generation', async () => {
+      jest.spyOn(anyTestFailed, 'default').mockResolvedValue(true);
+
+      await action.merge();
+
+      expect(context.archiveCommand.commands).toEqual([
+        {
+          command: 'archive',
+          options: {index: false},
+        },
+      ]);
+    });
+  });
 });