fixup! feat: Merge findings with local state

packages/scanner/src/cli/ci/command.ts

@@ -30,6 +30,7 @@ import fail from '../fail';
 import codeVersionArgs from '../codeVersionArgs';
 import { handleWorkingDirectory } from '../handleWorkingDirectory';
 import { stateFileNameArg } from '../triage/stateFileNameArg';
+import assert from 'assert';
 
 export default {
   command: 'ci',
@@ -95,7 +96,8 @@ export default {
       );
 
     await validateFile('directory', appmapDir!);
-    const appId = await resolveAppId(appIdArg, appmapDir);
+    const appId = await resolveAppId(appIdArg, appmapDir, true);
+    assert(appId);
 
     const glob = promisify(globCallback);
     const files = await glob(`${appmapDir}/**/*.appmap.json`);

packages/scanner/src/cli/merge/command.ts

@@ -5,6 +5,7 @@ import { merge as mergeScannerJob } from '../../integration/appland/scannerJob/m
 import resolveAppId from '../resolveAppId';
 import updateCommitStatus from '../updateCommitStatus';
 import fail from '../fail';
+import assert from 'assert';
 
 export default {
   command: 'merge <merge-key>',
@@ -45,7 +46,8 @@ export default {
       verbose(true);
     }
 
-    const appId = await resolveAppId(appIdArg, '.');
+    const appId = await resolveAppId(appIdArg, '.', true);
+    assert(appId);
 
     const mergeResults = await mergeScannerJob(appId, mergeKey);
     console.warn(`Merged results to ${mergeResults.url}`);

packages/scanner/src/cli/resolveAppId.ts

@@ -34,16 +34,23 @@ async function resolveAppId(
 
 export default async function (
   appIdArg: string | undefined,
-  appMapDir: string | undefined
-): Promise<string> {
+  appMapDir: string | undefined,
+  mustExist = false
+): Promise<string | undefined> {
   const appId = await resolveAppId(appIdArg, appMapDir);
   if (!appId) throw new ValidationError('App was not provided and could not be resolved');
 
   const appExists = await new App(appId).exists();
   if (!appExists) {
-    throw new ValidationError(
-      `App "${appId}" is not valid or does not exist.\nPlease fix the app name in the appmap.yml file, or override it with the --app option.`
+    if (mustExist) {
+      throw new ValidationError(
+        `App "${appId}" is not valid or does not exist. Please fix the app name in the appmap.yml file, or override it with the --app option.`
+      );
+    }
+    console.warn(
+      `App "${appId}" does not exist on the AppMap Server. If this is unexpected, provide the correct app name in the appmap.yml file, or override it with the --app option.`
     );
+    return;
   }
 
   return appId;

packages/scanner/src/cli/scan/command.ts

@@ -80,8 +80,7 @@ export default {
       appmapDir = (await appmapDirFromConfig()) || '.';
     }
 
-    let appId = appIdArg;
-    if (!watch) appId = await resolveAppId(appIdArg, appmapDir);
+    const appId = await resolveAppId(appIdArg, appmapDir);
 
     if (watch) {
       assert(appmapDir);

packages/scanner/src/cli/scan/scanner.ts

@@ -17,7 +17,7 @@ export default class Scanner {
   constructor(public configuration: Configuration, public files: string[]) {}
 
   async scan(skipErrors = false): Promise<ScanResults> {
-    await loadClientConfiguration();
+    loadClientConfiguration();
 
     const checks = await loadConfig(this.configuration);
     const { appMapMetadata, findings } = await scan(this.files, checks, skipErrors);
@@ -29,8 +29,11 @@ export default class Scanner {
     appIdArg?: string,
     appMapDir?: string
   ): Promise<FindingStatusListItem[]> {
+    let findingStatus: FindingStatusListItem[] = [];
+
     const appId = await resolveAppId(appIdArg, appMapDir);
-    const findingStatus = await new App(appId).listFindingStatus();
+    if (appId) findingStatus = await new App(appId).listFindingStatus();
+
     const clientFindingsState = await loadFindingsState(stateFileName);
     Object.keys(clientFindingsState).forEach((state): void => {
       const items = clientFindingsState[state as FindingState];

packages/scanner/src/cli/upload/command.ts

@@ -14,6 +14,7 @@ import upload from '../upload';
 import codeVersionArgs from '../codeVersionArgs';
 import { ValidationError } from '../../errors';
 import { handleWorkingDirectory } from '../handleWorkingDirectory';
+import assert from 'assert';
 
 export default {
   command: 'upload',
@@ -71,6 +72,7 @@ export default {
 
     await validateFile('directory', appmapDir!);
     const appId = await resolveAppId(appIdArg, appmapDir);
+    assert(appId);
 
     const scanResults = JSON.parse((await readFile(reportFile)).toString()) as ScanResults;
     const uploadResponse = await upload(

packages/scanner/test/cli/commands.spec.ts

@@ -29,30 +29,29 @@ describe('commands', () => {
   });
 
   describe('scan --watch', () => {
-    it('does not attempt to resolve an app ID', async () => {
+    let watcher: watchScan.Watcher | undefined;
+
+    afterEach(() => {
+      if (watcher) {
+        const closer = watcher.close();
+        watcher = undefined;
+        return closer;
+      }
+    });
+
+    it('resolves appId, but its absence is benign', async () => {
       // Prevent the watcher from running indefinitely
       jest.spyOn(watchScan, 'default').mockResolvedValue();
 
       const spy = jest.spyOn(resolveAppId, 'default');
 
       try {
         await ScanCommand.handler({ ...defaultArguments, watch: true });
-      } catch {
-        // Do nothing.
-        // We don't want exceptions, we just want to know if our stub was called.
+      } catch (e) {
+        expect(e).not.toBeTruthy();
       }
 
-      expect(spy).not.toBeCalled();
-    });
-
-    let watcher: watchScan.Watcher | undefined;
-
-    afterEach(() => {
-      if (watcher) {
-        const closer = watcher.close();
-        watcher = undefined;
-        return closer;
-      }
+      expect(spy).toBeCalled();
     });
 
     it('work correctly even if the appmap directory does not initially exist', () =>

packages/scanner/test/cli/scan.spec.ts

@@ -59,47 +59,32 @@ function runCommand(options: CommandOptions): Promise<void> {
 }
 
 describe('scan', () => {
-  it('errors with default options and without AppMap server API key', async () => {
-    delete process.env.APPLAND_API_KEY;
-    try {
-      await runCommand(StandardOneShotScanOptions);
-      throw new Error(`Expected this command to fail`);
-    } catch (err) {
-      expect((err as any).toString()).toMatch(/No API key available for AppMap server/);
-    }
+  describe('without AppMap credentials', () => {
+    it('reports missing credentials error', async () => {
+      delete process.env.APPLAND_API_KEY;
+      delete process.env.APPMAP_API_KEY;
+      try {
+        await runCommand(StandardOneShotScanOptions);
+        throw new Error(`Expected this command to fail`);
+      } catch (err) {
+        expect((err as any).toString()).toMatch(/No API key available for AppMap server/);
+      }
+    });
   });
 
-  async function checkScan(options: CommandOptions): Promise<void> {
-    await runCommand(options);
-
-    const scanResults = JSON.parse(readFileSync(ReportFile).toString()) as ScanResults;
-    expect(scanResults.summary).toBeTruthy();
-    const appMapMetadata = scanResults.summary.appMapMetadata;
-    expect(appMapMetadata.apps).toEqual(['spring-petclinic']);
-    const checks = scanResults.configuration.checks;
-    ['http-500', 'n-plus-one-query'].forEach((rule) =>
-      expect(checks.map((check) => check.rule)).toContain(rule)
-    );
-    expect(Object.keys(scanResults).sort()).toEqual([
-      'appMapMetadata',
-      'checks',
-      'configuration',
-      'findings',
-      'summary',
-    ]);
-  }
-
-  it('errors when the provided appId is not valid', async () => {
-    nock('http://localhost:3000').head(`/api/${AppId}`).reply(404);
-
-    try {
-      await runCommand(StandardOneShotScanOptions);
-      throw new Error(`Expected this command to fail`);
-    } catch (e) {
-      expect((e as any).message).toMatch(
-        /App "myorg\/sample_app_6th_ed" is not valid or does not exist./
-      );
-    }
+  describe('when the provided appId is not valid', () => {
+    it('reports missing or unknown app error ', async () => {
+      nock('http://localhost:3000').head(`/api/${AppId}`).reply(404);
+
+      try {
+        await runCommand(StandardOneShotScanOptions);
+        throw new Error(`Expected this command to fail`);
+      } catch (e) {
+        expect((e as any).message).toMatch(
+          /App "myorg\/sample_app_6th_ed" is not valid or does not exist./
+        );
+      }
+    });
   });
 
   it('integrates server finding status', async () => {
@@ -115,6 +100,26 @@ describe('scan', () => {
   it('skips when encountering a bad file in a directory', async () =>
     tmp.withDir(
       async ({ path }) => {
+        async function checkScan(options: CommandOptions): Promise<void> {
+          await runCommand(options);
+
+          const scanResults = JSON.parse(readFileSync(ReportFile).toString()) as ScanResults;
+          expect(scanResults.summary).toBeTruthy();
+          const appMapMetadata = scanResults.summary.appMapMetadata;
+          expect(appMapMetadata.apps).toEqual(['spring-petclinic']);
+          const checks = scanResults.configuration.checks;
+          ['http-500', 'n-plus-one-query'].forEach((rule) =>
+            expect(checks.map((check) => check.rule)).toContain(rule)
+          );
+          expect(Object.keys(scanResults).sort()).toEqual([
+            'appMapMetadata',
+            'checks',
+            'configuration',
+            'findings',
+            'summary',
+          ]);
+        }
+
         await copyFile(StandardOneShotScanOptions.appmapFile, join(path, 'good.appmap.json'));
         await writeFile(join(path, 'bad.appmap.json'), 'bad json');
 
@@ -129,38 +134,42 @@ describe('scan', () => {
       { unsafeCleanup: true }
     ));
 
-  it('errors when no good files were found', async () =>
-    tmp.withDir(
-      async ({ path }) => {
-        await writeFile(join(path, 'bad.appmap.json'), 'bad json');
+  describe('when the only existing AppMap(s) in a directory is invalid', () => {
+    it('reports a processing error', async () =>
+      tmp.withDir(
+        async ({ path }) => {
+          await writeFile(join(path, 'bad.appmap.json'), 'bad json');
+
+          const options: CommandOptions = {
+            ...StandardOneShotScanOptions,
+            appmapDir: path,
+          };
+          delete options.appmapFile;
+
+          expect.assertions(1);
+          return runCommand(options).catch((e: Error) => {
+            expect(e.message).toMatch(/Error processing/);
+          });
+        },
+        { unsafeCleanup: true }
+      ));
+  });
 
-        const options: CommandOptions = {
+  describe('when an invalid AppMap file is explicitly provided', () => {
+    it('reports a processing error', async () =>
+      tmp.withFile(async ({ path }) => {
+        await writeFile(path, 'bad json');
+        const options = {
           ...StandardOneShotScanOptions,
-          appmapDir: path,
+          all: true,
+          appmapFile: [path, StandardOneShotScanOptions.appmapFile],
         };
-        delete options.appmapFile;
-
         expect.assertions(1);
         return runCommand(options).catch((e: Error) => {
           expect(e.message).toMatch(/Error processing/);
         });
-      },
-      { unsafeCleanup: true }
-    ));
-
-  it('errors when a bad file is explicitly provided', async () =>
-    tmp.withFile(async ({ path }) => {
-      await writeFile(path, 'bad json');
-      const options = {
-        ...StandardOneShotScanOptions,
-        all: true,
-        appmapFile: [path, StandardOneShotScanOptions.appmapFile],
-      };
-      expect.assertions(1);
-      return runCommand(options).catch((e: Error) => {
-        expect(e.message).toMatch(/Error processing/);
-      });
-    }));
+      }));
+  });
 
   describe('watch mode', () => {
     let watcher: Watcher | undefined;