diff --git a/packages/ado-extension/src/ado-extension.ts b/packages/ado-extension/src/ado-extension.ts index 0028c02ce..274d93420 100644 --- a/packages/ado-extension/src/ado-extension.ts +++ b/packages/ado-extension/src/ado-extension.ts @@ -4,13 +4,14 @@ import 'reflect-metadata'; import './module-name-mapper'; +import { ExitCode } from '@accessibility-insights-action/shared'; import { Logger } from '@accessibility-insights-action/shared'; import { hookStderr } from '@accessibility-insights-action/shared'; import { hookStdout } from '@accessibility-insights-action/shared'; import { Scanner } from '@accessibility-insights-action/shared'; import { setupIocContainer } from './ioc/setup-ioc-container'; -export function runScan() { +export function runScan(): void { (async () => { hookStderr(); hookStdout(); @@ -20,9 +21,9 @@ export function runScan() { await logger.setup(); const scanner = container.get(Scanner); - await scanner.scan(); + process.exit((await scanner.scan()) ? ExitCode.ScanCompletedNoUserActionIsNeeded : ExitCode.ScanCompletedUserActionIsNeeded); })().catch((error) => { - console.log('Exception thrown in extension: ', error); - process.exit(1); + console.log('##[error][Exception] Exception thrown in extension: ', error); + process.exit(ExitCode.ScanFailedToComplete); }); } diff --git a/packages/ado-extension/src/install-runtime-dependencies.ts b/packages/ado-extension/src/install-runtime-dependencies.ts index 20e6d4ebc..06f55eaab 100644 --- a/packages/ado-extension/src/install-runtime-dependencies.ts +++ b/packages/ado-extension/src/install-runtime-dependencies.ts @@ -3,7 +3,7 @@ import { execSync } from 'child_process'; -export function installRuntimeDependencies() { +export function installRuntimeDependencies(): void { console.log('##[group]Installing runtime dependencies'); execSync('yarn install --prod --ignore-engines --frozen-lockfile', { stdio: 'inherit', diff --git a/packages/ado-extension/src/ioc/setup-ioc-container.ts b/packages/ado-extension/src/ioc/setup-ioc-container.ts index 5740cc7a2..9305029d9 100644 --- a/packages/ado-extension/src/ioc/setup-ioc-container.ts +++ b/packages/ado-extension/src/ioc/setup-ioc-container.ts @@ -21,7 +21,6 @@ export function setupIocContainer(container = new inversify.Container({ autoBind container .bind(iocTypes.ProgressReporters) .toDynamicValue((context) => { - // Note: Keep the WorkflowEnforcer creator last in the array! return [context.container.get(AdoConsoleCommentCreator), context.container.get(WorkflowEnforcer)]; }) .inSingletonScope(); diff --git a/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.spec.ts b/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.spec.ts index 1d099928a..c135dc33b 100644 --- a/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.spec.ts +++ b/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.spec.ts @@ -23,7 +23,7 @@ describe(AdoConsoleCommentCreator, () => { describe('constructor', () => { it('should initialize', () => { - adoConsoleCommentCreator = buildAdoConsoleCommentCreatorWithMocks(); + buildAdoConsoleCommentCreatorWithMocks(); verifyAllMocks(); }); @@ -64,16 +64,31 @@ describe(AdoConsoleCommentCreator, () => { describe('failRun', () => { it('does nothing interesting', async () => { - const message = 'message'; - adoConsoleCommentCreator = buildAdoConsoleCommentCreatorWithMocks(); + const adoConsoleCommentCreator = buildAdoConsoleCommentCreatorWithMocks(); - await adoConsoleCommentCreator.failRun(message); + await adoConsoleCommentCreator.failRun(); verifyAllMocks(); }); }); - const buildAdoConsoleCommentCreatorWithMocks = () => + describe('didScanSucceed', () => { + it('returns true by default', async () => { + const adoConsoleCommentCreator = buildAdoConsoleCommentCreatorWithMocks(); + + await expect(adoConsoleCommentCreator.didScanSucceed()).resolves.toBe(true); + }); + + it('returns true after failRun() is called', async () => { + const adoConsoleCommentCreator = buildAdoConsoleCommentCreatorWithMocks(); + + await adoConsoleCommentCreator.failRun(); + + await expect(adoConsoleCommentCreator.didScanSucceed()).resolves.toBe(true); + }); + }); + + const buildAdoConsoleCommentCreatorWithMocks = (): AdoConsoleCommentCreator => new AdoConsoleCommentCreator(adoTaskConfigMock.object, reportMarkdownConvertorMock.object, loggerMock.object); const verifyAllMocks = () => { diff --git a/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.ts b/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.ts index 0d971caaf..7dab0786a 100644 --- a/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.ts +++ b/packages/ado-extension/src/progress-reporter/console/ado-console-comment-creator.ts @@ -39,8 +39,8 @@ export class AdoConsoleCommentCreator extends ProgressReporter { } // eslint-disable-next-line @typescript-eslint/no-unused-vars - public async failRun(message: string): Promise { - // This gets handled in the WorkflowEnforcer + public async failRun(): Promise { + // We don't do anything for failed runs } private getBaselineInfo(baselineEvaluation?: BaselineEvaluation): BaselineInfo { diff --git a/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.spec.ts b/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.spec.ts index ad1c2b5df..5bb806076 100644 --- a/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.spec.ts +++ b/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.spec.ts @@ -8,13 +8,16 @@ import { CombinedReportParameters } from 'accessibility-insights-report'; import { BaselineEvaluation, BaselineFileContent } from 'accessibility-insights-scan'; import { WorkflowEnforcer } from './workflow-enforcer'; +import { Logger } from '@accessibility-insights-action/shared'; describe(WorkflowEnforcer, () => { let adoTaskConfigMock: IMock; + let loggerMock: IMock; let workflowEnforcer: WorkflowEnforcer; beforeEach(() => { adoTaskConfigMock = Mock.ofType(undefined, MockBehavior.Strict); + loggerMock = Mock.ofType(undefined, MockBehavior.Strict); }); describe('constructor', () => { @@ -26,7 +29,7 @@ describe(WorkflowEnforcer, () => { }); describe('completeRun', () => { - it('throws correct error if accessibility error occurred', async () => { + it('logs correct error if accessibility error occurred', async () => { const reportStub = { results: { urlResults: { @@ -37,17 +40,16 @@ describe(WorkflowEnforcer, () => { const baselineEvaluationStub = {} as BaselineEvaluation; setupFailOnAccessibilityError(true); + setupLoggerWithErrorMessage('An accessibility error was found and you specified the "failOnAccessibilityError" flag.'); - workflowEnforcer = buildWorkflowEnforcerWithMocks(); + const workflowEnforcer = buildWorkflowEnforcerWithMocks(); - await expect(workflowEnforcer.completeRun(reportStub, baselineEvaluationStub)).rejects.toThrowError( - 'Failed Accessibility Error', - ); + await workflowEnforcer.completeRun(reportStub, baselineEvaluationStub); verifyAllMocks(); }); - it('throws correct error if baseline needs to be updated', async () => { + it('logs correct error if baseline needs to be updated', async () => { const reportStub = {} as CombinedReportParameters; const baselineEvaluationStub = { suggestedBaselineUpdate: {} as BaselineFileContent, @@ -55,12 +57,11 @@ describe(WorkflowEnforcer, () => { setupFailOnAccessibilityError(false); setupBaselineFileParameterExists(); + setupLoggerWithErrorMessage('The baseline file does not match scan results.'); - workflowEnforcer = buildWorkflowEnforcerWithMocks(); + const workflowEnforcer = buildWorkflowEnforcerWithMocks(); - await expect(workflowEnforcer.completeRun(reportStub, baselineEvaluationStub)).rejects.toThrowError( - 'Failed: The baseline file does not match scan results. If this is a PR, check the PR comments.', - ); + await workflowEnforcer.completeRun(reportStub, baselineEvaluationStub); verifyAllMocks(); }); @@ -72,7 +73,7 @@ describe(WorkflowEnforcer, () => { setupFailOnAccessibilityError(false); setupBaselineFileParameterExists(); - workflowEnforcer = buildWorkflowEnforcerWithMocks(); + const workflowEnforcer = buildWorkflowEnforcerWithMocks(); await workflowEnforcer.completeRun(reportStub, baselineEvaluationStub); @@ -92,16 +93,20 @@ describe(WorkflowEnforcer, () => { }); }); - describe('failRun', () => { - it('reject promise with matching error', async () => { - workflowEnforcer = buildWorkflowEnforcerWithMocks(); + describe('didScanSucceed', () => { + it('returns true by default', async () => { + const workflowEnforcer = buildWorkflowEnforcerWithMocks(); + await expect(workflowEnforcer.didScanSucceed()).resolves.toBe(true); + }); - await expect(workflowEnforcer.failRun('message')).rejects.toThrowError('message'); - verifyAllMocks(); + it('returns false after failRun() is called', async () => { + const workflowEnforcer = buildWorkflowEnforcerWithMocks(); + await workflowEnforcer.failRun(); + await expect(workflowEnforcer.didScanSucceed()).resolves.toBe(false); }); }); - const buildWorkflowEnforcerWithMocks = () => new WorkflowEnforcer(adoTaskConfigMock.object); + const buildWorkflowEnforcerWithMocks = () => new WorkflowEnforcer(adoTaskConfigMock.object, loggerMock.object); const verifyAllMocks = () => { adoTaskConfigMock.verifyAll(); @@ -120,4 +125,8 @@ describe(WorkflowEnforcer, () => { .returns(() => 'baseline-file') .verifiable(Times.atLeastOnce()); }; + + const setupLoggerWithErrorMessage = (message: string) => { + loggerMock.setup((o) => o.logError(message)).verifiable(Times.once()); + }; }); diff --git a/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.ts b/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.ts index 2dd781528..0f72eff11 100644 --- a/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.ts +++ b/packages/ado-extension/src/progress-reporter/enforcement/workflow-enforcer.ts @@ -1,16 +1,17 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. -import { AdoIocTypes } from '../../ioc/ado-ioc-types'; import { ADOTaskConfig } from '../../task-config/ado-task-config'; import { inject, injectable } from 'inversify'; -import { ProgressReporter } from '@accessibility-insights-action/shared'; +import { Logger, ProgressReporter } from '@accessibility-insights-action/shared'; import { CombinedReportParameters } from 'accessibility-insights-report'; import { BaselineEvaluation } from 'accessibility-insights-scan'; @injectable() export class WorkflowEnforcer extends ProgressReporter { - constructor(@inject(ADOTaskConfig) private readonly adoTaskConfig: ADOTaskConfig) { + private scanSucceeded = true; + + constructor(@inject(ADOTaskConfig) private readonly adoTaskConfig: ADOTaskConfig, @inject(Logger) private readonly logger: Logger) { super(); } @@ -20,24 +21,35 @@ export class WorkflowEnforcer extends ProgressReporter { // eslint-disable-next-line @typescript-eslint/require-await public async completeRun(combinedReportResult: CombinedReportParameters, baselineEvaluation?: BaselineEvaluation): Promise { - this.failIfAccessibilityErrorExists(combinedReportResult); - this.failIfBaselineNeedsUpdating(baselineEvaluation); + if (!(await this.failIfAccessibilityErrorExists(combinedReportResult))) { + await this.failIfBaselineNeedsUpdating(baselineEvaluation); + } } // eslint-disable-next-line @typescript-eslint/require-await - public async failRun(message: string): Promise { - throw new Error(message); + public async failRun(): Promise { + this.scanSucceeded = false; + } + + public didScanSucceed(): Promise { + return Promise.resolve(this.scanSucceeded); } - private failIfAccessibilityErrorExists(combinedReportResult: CombinedReportParameters): void { + private async failIfAccessibilityErrorExists(combinedReportResult: CombinedReportParameters): Promise { if (this.adoTaskConfig.getFailOnAccessibilityError() && combinedReportResult.results.urlResults.failedUrls > 0) { - throw new Error('Failed Accessibility Error'); + this.logger.logError('An accessibility error was found and you specified the "failOnAccessibilityError" flag.'); + await this.failRun(); + return true; } + return false; } - private failIfBaselineNeedsUpdating(baselineEvaluation?: BaselineEvaluation): void { + private async failIfBaselineNeedsUpdating(baselineEvaluation?: BaselineEvaluation): Promise { if (baselineEvaluation && this.adoTaskConfig.getBaselineFile() && baselineEvaluation.suggestedBaselineUpdate) { - throw new Error('Failed: The baseline file does not match scan results. If this is a PR, check the PR comments.'); + this.logger.logError('The baseline file does not match scan results.'); + await this.failRun(); + return true; } + return false; } } diff --git a/packages/gh-action/src/console/console-comment-creator.spec.ts b/packages/gh-action/src/console/console-comment-creator.spec.ts index a0567d94e..2afc07107 100644 --- a/packages/gh-action/src/console/console-comment-creator.spec.ts +++ b/packages/gh-action/src/console/console-comment-creator.spec.ts @@ -32,10 +32,14 @@ describe(ConsoleCommentCreator, () => { }); }); - describe('failRun', () => { - it('throws error', async () => { - const errorMessage = 'some error message'; - await expect(testSubject.failRun(errorMessage)).rejects.toBe(errorMessage); + describe('didScanSucceed', () => { + it('returns true by default', async () => { + await expect(testSubject.didScanSucceed()).resolves.toBe(true); + }); + + it('returns false after failRun() is called', async () => { + await testSubject.failRun(); + await expect(testSubject.didScanSucceed()).resolves.toBe(false); }); }); diff --git a/packages/gh-action/src/console/console-comment-creator.ts b/packages/gh-action/src/console/console-comment-creator.ts index afc53cc37..1862edf03 100644 --- a/packages/gh-action/src/console/console-comment-creator.ts +++ b/packages/gh-action/src/console/console-comment-creator.ts @@ -7,6 +7,8 @@ import { CombinedReportParameters } from 'accessibility-insights-report'; @injectable() export class ConsoleCommentCreator extends ProgressReporter { + private scanSucceeded = true; + constructor( @inject(ReportMarkdownConvertor) private readonly reportMarkdownConvertor: ReportMarkdownConvertor, @inject(Logger) private readonly logger: Logger, @@ -27,7 +29,11 @@ export class ConsoleCommentCreator extends ProgressReporter { } // eslint-disable-next-line @typescript-eslint/require-await - public async failRun(message: string): Promise { - throw message; + public async failRun(): Promise { + this.scanSucceeded = false; + } + + public didScanSucceed(): Promise { + return Promise.resolve(this.scanSucceeded); } } diff --git a/packages/gh-action/src/index.ts b/packages/gh-action/src/index.ts index b688b227c..011813ae2 100644 --- a/packages/gh-action/src/index.ts +++ b/packages/gh-action/src/index.ts @@ -3,6 +3,7 @@ import 'reflect-metadata'; import './module-name-mapper'; +import { ExitCode } from '@accessibility-insights-action/shared'; import { Logger } from '@accessibility-insights-action/shared'; import { hookStderr } from '@accessibility-insights-action/shared'; import { hookStdout } from '@accessibility-insights-action/shared'; @@ -18,8 +19,8 @@ import { setupIocContainer } from './ioc/setup-ioc-container'; await logger.setup(); const scanner = container.get(Scanner); - await scanner.scan(); + process.exit((await scanner.scan()) ? ExitCode.ScanCompletedNoUserActionIsNeeded : ExitCode.ScanCompletedUserActionIsNeeded); })().catch((error) => { - console.log('Exception thrown in action: ', error); - process.exit(1); + console.log('::error::[Exception] Exception thrown in extension: ', error); + process.exit(ExitCode.ScanFailedToComplete); }); diff --git a/packages/shared/src/exit-code.ts b/packages/shared/src/exit-code.ts new file mode 100644 index 000000000..271da605e --- /dev/null +++ b/packages/shared/src/exit-code.ts @@ -0,0 +1,8 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +export const ExitCode = { + ScanCompletedNoUserActionIsNeeded: 0, + ScanCompletedUserActionIsNeeded: 1, + ScanFailedToComplete: 2, +}; diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index 855b1a7a5..9a15662b9 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -15,3 +15,4 @@ export { BaselineInfo } from './baseline-info'; export { ArtifactsInfoProvider } from './artifacts-info-provider'; export { hookStdout } from './output-hooks/hook-stdout'; export { hookStderr } from './output-hooks/hook-stderr'; +export { ExitCode } from './exit-code'; diff --git a/packages/shared/src/progress-reporter/all-progress-reporter.spec.ts b/packages/shared/src/progress-reporter/all-progress-reporter.spec.ts index 230beb1f9..2d2b61c79 100644 --- a/packages/shared/src/progress-reporter/all-progress-reporter.spec.ts +++ b/packages/shared/src/progress-reporter/all-progress-reporter.spec.ts @@ -23,7 +23,10 @@ describe(AllProgressReporter, () => { async completeRun(combinedReportResult: CombinedReportParameters, baselineEvaluation?: BaselineEvaluation): Promise { throw failingReporterError; }, - async failRun(message: string): Promise { + async failRun(): Promise { + throw failingReporterError; + }, + async didScanSucceed(): Promise { throw failingReporterError; }, } as ProgressReporter; @@ -123,42 +126,103 @@ describe(AllProgressReporter, () => { describe('failRun', () => { it('should invoke all reporters', async () => { - testSubject = new AllProgressReporter([progressReporterMock.object]); - const error = 'scan error'; + testSubject = new AllProgressReporter([progressReporterMock.object, progressReporterMock.object]); executeOnReporter((reporter) => { reporter - .setup((p) => p.failRun(error)) + .setup((p) => p.failRun()) .returns(() => Promise.resolve()) - .verifiable(Times.once()); + .verifiable(Times.exactly(2)); }); - await testSubject.failRun(error); + await testSubject.failRun(); }); it('should invoke subsequent reporters even if the first one throws', async () => { testSubject = new AllProgressReporter([failingReporter, progressReporterMock.object]); - const error = 'scan error'; executeOnReporter((reporter) => { reporter - .setup((p) => p.failRun(error)) + .setup((p) => p.failRun()) .returns(() => Promise.resolve()) .verifiable(Times.once()); }); // The error from the first reporter should be rethrown, but we should still see the call to the second reporter - await expect(testSubject.failRun(error)).rejects.toThrowError(failingReporterError); + await expect(testSubject.failRun()).rejects.toThrowError(failingReporterError); }); it('should rethrow an AggregateError if multiple reporters throw', async () => { testSubject = new AllProgressReporter([failingReporter, failingReporter]); - const error = 'scan error'; // The error from the first reporter should be rethrown, but we should still see the call to the second reporter - await expect(testSubject.failRun(error)).rejects.toThrowErrorMatchingInlineSnapshot(` - "Multiple progress reporters encountered Errors - error from failingReporter - error from failingReporter" - `); + await expect(testSubject.failRun()).rejects.toThrowErrorMatchingInlineSnapshot(` + "Multiple progress reporters encountered Errors + error from failingReporter + error from failingReporter" + `); + }); + }); + + describe('didScanSucceed', () => { + it('should invoke all reporters', async () => { + testSubject = new AllProgressReporter([progressReporterMock.object, progressReporterMock.object]); + executeOnReporter((reporter) => { + reporter + .setup((p) => p.didScanSucceed()) + .returns(() => Promise.resolve(true)) + .verifiable(Times.exactly(2)); + }); + + await testSubject.didScanSucceed(); + }); + + it('should invoke subsequent reporters even if the first one throws', async () => { + testSubject = new AllProgressReporter([failingReporter, progressReporterMock.object]); + executeOnReporter((reporter) => { + reporter + .setup((p) => p.didScanSucceed()) + .returns(() => Promise.resolve(true)) + .verifiable(Times.once()); + }); + + // The error from the first reporter should be rethrown, but we should still see the call to the second reporter + await expect(testSubject.didScanSucceed()).rejects.toThrowError(failingReporterError); + }); + + it('should rethrow an AggregateError if multiple reporters throw', async () => { + testSubject = new AllProgressReporter([failingReporter, failingReporter]); + + // The error from the first reporter should be rethrown, but we should still see the call to the second reporter + await expect(testSubject.didScanSucceed()).rejects.toThrowErrorMatchingInlineSnapshot(` + "Multiple progress reporters encountered Errors + error from failingReporter + error from failingReporter" + `); + }); + + it('should return true if all reporters return true', async () => { + testSubject = new AllProgressReporter([progressReporterMock.object, progressReporterMock.object]); + executeOnReporter((reporter) => { + reporter + .setup((p) => p.didScanSucceed()) + .returns(() => Promise.resolve(true)) + .verifiable(Times.exactly(2)); + }); + + await expect(testSubject.didScanSucceed()).resolves.toBe(true); + }); + + it('should return false if any reporters return false', async () => { + const returnValues = [false, true]; + let index = 0; + testSubject = new AllProgressReporter([progressReporterMock.object, progressReporterMock.object]); + executeOnReporter((reporter) => { + reporter + .setup((p) => p.didScanSucceed()) + .returns(() => Promise.resolve(returnValues[index++])) + .verifiable(Times.exactly(2)); + }); + + await expect(testSubject.didScanSucceed()).resolves.toBe(false); }); }); diff --git a/packages/shared/src/progress-reporter/all-progress-reporter.ts b/packages/shared/src/progress-reporter/all-progress-reporter.ts index 2fd30974a..dc42d1c81 100644 --- a/packages/shared/src/progress-reporter/all-progress-reporter.ts +++ b/packages/shared/src/progress-reporter/all-progress-reporter.ts @@ -26,8 +26,20 @@ export class AllProgressReporter extends ProgressReporter { } } - public async failRun(message: string): Promise { - await this.execute((r) => r.failRun(message)); + public async failRun(): Promise { + await this.execute((r) => r.failRun()); + } + + public async didScanSucceed(): Promise { + let allReportersSucceeded = true; + + await this.execute(async (r) => { + if (!(await r.didScanSucceed())) { + allReportersSucceeded = false; + } + }); + + return Promise.resolve(allReportersSucceeded); } private async execute(callback: (reporter: ProgressReporter) => Promise): Promise { diff --git a/packages/shared/src/progress-reporter/progress-reporter.ts b/packages/shared/src/progress-reporter/progress-reporter.ts index b673a0950..3e8187dcb 100644 --- a/packages/shared/src/progress-reporter/progress-reporter.ts +++ b/packages/shared/src/progress-reporter/progress-reporter.ts @@ -19,7 +19,10 @@ export abstract class ProgressReporter { public abstract start(): Promise; public abstract completeRun(combinedReportResult: CombinedReportParameters, baselineEvaluation?: BaselineEvaluation): Promise; - public abstract failRun(message: string): Promise; + public abstract failRun(): Promise; + public didScanSucceed(): Promise { + return Promise.resolve(true); // Will be overridden in classes that use this + } protected async invoke(fn: () => Promise): Promise { return process.env.ACT !== 'true' ? fn() : Promise.resolve(undefined as T); diff --git a/packages/shared/src/scanner/scanner.spec.ts b/packages/shared/src/scanner/scanner.spec.ts index acaa8525f..eeaff5f20 100644 --- a/packages/shared/src/scanner/scanner.spec.ts +++ b/packages/shared/src/scanner/scanner.spec.ts @@ -2,7 +2,6 @@ // Licensed under the MIT License. import 'reflect-metadata'; -import * as util from 'util'; import { AICombinedReportDataConverter, AICrawler, @@ -35,8 +34,6 @@ describe(Scanner, () => { let promiseUtilsMock: IMock; let axeInfoMock: IMock; let combinedReportConverterMock: IMock; - let processStub: typeof process; - let exitMock: IMock<(code: number) => void>; let loggerMock: IMock; let crawlArgumentHandlerMock: IMock; let taskConfigMock: IMock; @@ -57,10 +54,6 @@ describe(Scanner, () => { promiseUtilsMock = Mock.ofType(PromiseUtils); axeInfoMock = Mock.ofType(); combinedReportConverterMock = Mock.ofType(); - exitMock = Mock.ofType<(_: number) => void>(); - processStub = { - exit: exitMock.object, - } as typeof process; loggerMock = Mock.ofType(Logger); crawlArgumentHandlerMock = Mock.ofType(); taskConfigMock = Mock.ofType(); @@ -75,7 +68,6 @@ describe(Scanner, () => { promiseUtilsMock.object, axeInfoMock.object, combinedReportConverterMock.object, - processStub, loggerMock.object, crawlArgumentHandlerMock.object, taskConfigMock.object, @@ -96,22 +88,23 @@ describe(Scanner, () => { }); describe('scan', () => { - it('performs expected steps in happy path with remote url', async () => { + it('performs expected steps in happy path with remote url and returns true', async () => { setupMocksForSuccessfulScan(); - setupWaitForPromisetoReturnOriginalPromise(); + setupWaitForPromiseToReturnOriginalPromise(); - await scanner.scan(); + const result = await scanner.scan(); + expect(result).toBe(true); verifyMocks(); }); - it('performs expected steps in happy path with local url (starts file server)', async () => { + it('performs expected steps in happy path with local url (starts file server) and returns true', async () => { scanArguments.url = ''; localFileServerMock.setup((m) => m.start()).returns((_) => Promise.resolve('localhost')); setupMocksForSuccessfulScan(); - setupWaitForPromisetoReturnOriginalPromise(); + setupWaitForPromiseToReturnOriginalPromise(); - await scanner.scan(); + await expect(scanner.scan()).resolves.toBe(true); verifyMocks(); localFileServerMock.verify((m) => m.start(), Times.once()); @@ -119,29 +112,28 @@ describe(Scanner, () => { it('passes BaselineEvaluation to ProgressReporter', async () => { setupMocksForSuccessfulScan({} as BaselineEvaluation); - setupWaitForPromisetoReturnOriginalPromise(); + setupWaitForPromiseToReturnOriginalPromise(); - await scanner.scan(); + await expect(scanner.scan()).resolves.toBe(true); verifyMocks(); }); - it('reports error when timeout occurs', async () => { + it('reports error when timeout occurs and returns false', async () => { const errorMessage = `Scan timed out after ${scanTimeoutMsec / 1000} seconds`; localFileServerMock.setup((m) => m.stop()).verifiable(Times.once()); loggerMock.setup((lm) => lm.logError(errorMessage)).verifiable(Times.once()); - exitMock.setup((em) => em(1)).verifiable(Times.once()); taskConfigMock .setup((m) => m.getScanTimeout()) .returns((_) => scanTimeoutMsec) .verifiable(Times.once()); setupWaitForPromiseToReturnTimeoutPromise(); - await scanner.scan(); + await expect(scanner.scan()).resolves.toBe(false); verifyMocks(); }); - it('should trackException on error', async () => { + it('should trackException and return false after an Error is thrown', async () => { const errorMessage = 'some err'; const error = new Error(errorMessage); @@ -152,12 +144,12 @@ describe(Scanner, () => { .setup((lm) => lm.trackExceptionAny(error, `An error occurred while scanning website page undefined`)) .verifiable(Times.once()); loggerMock.setup((lm) => lm.logInfo(`Accessibility scanning of URL undefined completed`)).verifiable(Times.once()); - progressReporterMock.setup((p) => p.failRun(util.inspect(error))).verifiable(Times.once()); + progressReporterMock.setup((p) => p.failRun()).verifiable(Times.once()); localFileServerMock.setup((m) => m.stop()).verifiable(Times.once()); - setupWaitForPromisetoReturnOriginalPromise(); + setupWaitForPromiseToReturnOriginalPromise(); - await scanner.scan(); + await expect(scanner.scan()).resolves.toBe(false); verifyMocks(); }); @@ -172,6 +164,10 @@ describe(Scanner, () => { .returns((_) => scanArguments.url) .verifiable(Times.once()); progressReporterMock.setup((p) => p.start()).verifiable(Times.once()); + progressReporterMock + .setup((m) => m.didScanSucceed()) + .returns(() => Promise.resolve(true)) + .verifiable(Times.once()); crawlArgumentHandlerMock .setup((m) => m.processScanArguments(It.isAny())) .returns((_) => scanArguments) @@ -220,7 +216,7 @@ describe(Scanner, () => { localFileServerMock.setup((lfs) => lfs.stop()).verifiable(); } - function setupWaitForPromisetoReturnOriginalPromise(): void { + function setupWaitForPromiseToReturnOriginalPromise(): void { promiseUtilsMock .setup((s) => s.waitFor(It.isAny(), scanTimeoutMsec, It.isAny())) // eslint-disable-next-line @typescript-eslint/require-await diff --git a/packages/shared/src/scanner/scanner.ts b/packages/shared/src/scanner/scanner.ts index ad46e3262..a48d62e03 100644 --- a/packages/shared/src/scanner/scanner.ts +++ b/packages/shared/src/scanner/scanner.ts @@ -11,7 +11,6 @@ import { BaselineFileUpdater, } from 'accessibility-insights-scan'; import { inject, injectable } from 'inversify'; -import * as util from 'util'; import { iocTypes } from '../ioc/ioc-types'; import { LocalFileServer } from '../local-file-server'; import { Logger } from '../logger/logger'; @@ -25,6 +24,8 @@ import { CrawlArgumentHandler } from './crawl-argument-handler'; import { TaskConfig } from '../task-config'; import { isEmpty } from 'lodash'; +export type ScanSucceededWithNoRequiredUserAction = boolean; + @injectable() export class Scanner { constructor( @@ -35,7 +36,6 @@ export class Scanner { @inject(PromiseUtils) private readonly promiseUtils: PromiseUtils, @inject(AxeInfo) private readonly axeInfo: AxeInfo, @inject(AICombinedReportDataConverter) private readonly combinedReportDataConverter: AICombinedReportDataConverter, - @inject(iocTypes.Process) protected readonly currentProcess: typeof process, @inject(Logger) private readonly logger: Logger, @inject(CrawlArgumentHandler) private readonly crawlArgumentHandler: CrawlArgumentHandler, @inject(iocTypes.TaskConfig) private readonly taskConfig: TaskConfig, @@ -44,16 +44,19 @@ export class Scanner { @inject(BaselineFileUpdater) private readonly baselineFileUpdater: BaselineFileUpdater, ) {} - public async scan(): Promise { + public async scan(): Promise { const scanTimeoutMsec = this.taskConfig.getScanTimeout(); - // eslint-disable-next-line @typescript-eslint/require-await - await this.promiseUtils.waitFor(this.invokeScan(), scanTimeoutMsec, async () => { - this.logger.logError(`Scan timed out after ${scanTimeoutMsec / 1000} seconds`); - this.currentProcess.exit(1); - }); + return this.promiseUtils.waitFor( + this.invokeScan(), + scanTimeoutMsec, + async (): Promise => { + this.logger.logError(`Scan timed out after ${scanTimeoutMsec / 1000} seconds`); + return Promise.resolve(false); + }, + ); } - private async invokeScan(): Promise { + private async invokeScan(): Promise { let scanArguments: ScanArguments; let localServerUrl: string; @@ -80,13 +83,16 @@ export class Scanner { await this.baselineFileUpdater.updateBaseline(scanArguments, combinedScanResult.baselineEvaluation); await this.allProgressReporter.completeRun(combinedReportParameters, combinedScanResult.baselineEvaluation); + return this.allProgressReporter.didScanSucceed(); } catch (error) { this.logger.trackExceptionAny(error, `An error occurred while scanning website page ${scanArguments?.url}`); - await this.allProgressReporter.failRun(util.inspect(error)); + await this.allProgressReporter.failRun(); } finally { this.fileServer.stop(); this.logger.logInfo(`Accessibility scanning of URL ${scanArguments?.url} completed`); } + + return Promise.resolve(false); } private getCombinedReportParameters(