diff --git a/packages/scanner/src/cli/ci/command.ts b/packages/scanner/src/cli/ci/command.ts index 84bdf35666..874a9b5cee 100644 --- a/packages/scanner/src/cli/ci/command.ts +++ b/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`); diff --git a/packages/scanner/src/cli/merge/command.ts b/packages/scanner/src/cli/merge/command.ts index b779b731cf..995199d568 100644 --- a/packages/scanner/src/cli/merge/command.ts +++ b/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 ', @@ -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}`); diff --git a/packages/scanner/src/cli/resolveAppId.ts b/packages/scanner/src/cli/resolveAppId.ts index a6cc5e8894..1423388459 100644 --- a/packages/scanner/src/cli/resolveAppId.ts +++ b/packages/scanner/src/cli/resolveAppId.ts @@ -34,16 +34,23 @@ async function resolveAppId( export default async function ( appIdArg: string | undefined, - appMapDir: string | undefined -): Promise { + appMapDir: string | undefined, + mustExist = false +): Promise { 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; diff --git a/packages/scanner/src/cli/scan/command.ts b/packages/scanner/src/cli/scan/command.ts index 9e8c86024e..2852e543b5 100644 --- a/packages/scanner/src/cli/scan/command.ts +++ b/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); diff --git a/packages/scanner/src/cli/scan/scanner.ts b/packages/scanner/src/cli/scan/scanner.ts index 469d405057..b87358559f 100644 --- a/packages/scanner/src/cli/scan/scanner.ts +++ b/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 { - 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 { + 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]; diff --git a/packages/scanner/src/cli/upload/command.ts b/packages/scanner/src/cli/upload/command.ts index d84131db51..49ba37cce1 100644 --- a/packages/scanner/src/cli/upload/command.ts +++ b/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( diff --git a/packages/scanner/test/cli/commands.spec.ts b/packages/scanner/test/cli/commands.spec.ts index a99c9b1b16..9a85b77ea9 100644 --- a/packages/scanner/test/cli/commands.spec.ts +++ b/packages/scanner/test/cli/commands.spec.ts @@ -29,7 +29,17 @@ 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(); @@ -37,22 +47,11 @@ describe('commands', () => { 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', () => diff --git a/packages/scanner/test/cli/scan.spec.ts b/packages/scanner/test/cli/scan.spec.ts index 7074f25a13..bc9fc5093a 100644 --- a/packages/scanner/test/cli/scan.spec.ts +++ b/packages/scanner/test/cli/scan.spec.ts @@ -59,47 +59,32 @@ function runCommand(options: CommandOptions): Promise { } 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 { - 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 { + 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;