refactor(docs-infra): use mockable logger (#25671)

Related discussion:
https://github.com/angular/angular/pull/23576#discussion_r187925949.

PR Close #25671

aio/aio-builds-setup/dockerbuild/scripts-js/lib/clean-up/build-cleaner.ts

@@ -5,12 +5,12 @@ import * as shell from 'shelljs';
 import {HIDDEN_DIR_PREFIX} from '../common/constants';
 import {GithubApi} from '../common/github-api';
 import {GithubPullRequests} from '../common/github-pull-requests';
-import {assertNotMissingOrEmpty, createLogger, getPrInfoFromDownloadPath} from '../common/utils';
+import {assertNotMissingOrEmpty, getPrInfoFromDownloadPath, Logger} from '../common/utils';
 
 // Classes
 export class BuildCleaner {
 
-  private logger = createLogger('BuildCleaner');
+  private logger = new Logger('BuildCleaner');
 
   // Constructor
   constructor(protected buildsDir: string, protected githubOrg: string, protected githubRepo: string,

aio/aio-builds-setup/dockerbuild/scripts-js/lib/common/utils.ts

@@ -74,12 +74,25 @@ export const getEnvVar = (name: string, isOptional = false): string => {
   return value || '';
 };
 
-export function createLogger(scope: string) {
-  const padding = ' '.repeat(20 - scope.length);
-  return {
-    error: (...args: any[]) => console.error(`[${new Date()}]`, `${scope}:${padding}`, ...args),
-    info: (...args: any[]) => console.info(`[${new Date()}]`, `${scope}:${padding}`, ...args),
-    log: (...args: any[]) => console.log(`[${new Date()}]`, `${scope}:${padding}`, ...args),
-    warn: (...args: any[]) => console.warn(`[${new Date()}]`, `${scope}:${padding}`, ...args),
-  };
+/**
+ * A basic logger implementation.
+ * Delegates to `console`, but prepends each message with the current date and specified scope (i.e caller).
+ */
+export class Logger {
+  private padding = ' '.repeat(20 - this.scope.length);
+
+  /**
+   * Create a new `Logger` instance for the specified `scope`.
+   * @param scope The logger's scope (added to all messages).
+   */
+  constructor(private scope: string) {}
+
+  public error(...args: any[]) { this.callMethod('error', args); }
+  public info(...args: any[]) { this.callMethod('info', args); }
+  public log(...args: any[]) { this.callMethod('log', args); }
+  public warn(...args: any[]) { this.callMethod('warn', args); }
+
+  private callMethod(method: 'error' | 'info' | 'log' | 'warn', args: any[]) {
+    console[method](`[${new Date()}]`, `${this.scope}:${this.padding}`, ...args);
+  }
 }

aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/build-creator.ts

@@ -5,14 +5,14 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as shell from 'shelljs';
 import {HIDDEN_DIR_PREFIX} from '../common/constants';
-import {assertNotMissingOrEmpty, computeShortSha, createLogger} from '../common/utils';
+import {assertNotMissingOrEmpty, computeShortSha, Logger} from '../common/utils';
 import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
 import {PreviewServerError} from './preview-error';
 
 // Classes
 export class BuildCreator extends EventEmitter {
 
-  private logger = createLogger('BuildCreator');
+  private logger = new Logger('BuildCreator');
 
   // Constructor
   constructor(protected buildsDir: string) {

aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/build-retriever.ts

@@ -4,7 +4,7 @@ import {dirname} from 'path';
 import {mkdir} from 'shelljs';
 import {promisify} from 'util';
 import {CircleCiApi} from '../common/circle-ci-api';
-import {assert, assertNotMissingOrEmpty, computeArtifactDownloadPath, createLogger} from '../common/utils';
+import {assert, assertNotMissingOrEmpty, computeArtifactDownloadPath, Logger} from '../common/utils';
 import {PreviewServerError} from './preview-error';
 
 export interface GithubInfo {
@@ -19,7 +19,7 @@ export interface GithubInfo {
  * A helper that can get information about builds and download build artifacts.
  */
 export class BuildRetriever {
-  private logger = createLogger('BuildRetriever');
+  private logger = new Logger('BuildRetriever');
   constructor(private api: CircleCiApi, private downloadSizeLimit: number, private downloadDir: string) {
     assert(downloadSizeLimit > 0, 'Invalid parameter "downloadSizeLimit" should be a number greater than 0.');
     assertNotMissingOrEmpty('downloadDir', downloadDir);

aio/aio-builds-setup/dockerbuild/scripts-js/lib/preview-server/preview-server-factory.ts

@@ -7,7 +7,7 @@ import {CircleCiApi} from '../common/circle-ci-api';
 import {GithubApi} from '../common/github-api';
 import {GithubPullRequests} from '../common/github-pull-requests';
 import {GithubTeams} from '../common/github-teams';
-import {assert, assertNotMissingOrEmpty, createLogger} from '../common/utils';
+import {assert, assertNotMissingOrEmpty, Logger} from '../common/utils';
 import {BuildCreator} from './build-creator';
 import {ChangedPrVisibilityEvent, CreatedBuildEvent} from './build-events';
 import {BuildRetriever} from './build-retriever';
@@ -32,7 +32,7 @@ export interface PreviewServerConfig {
   trustedPrLabel: string;
 }
 
-const logger = createLogger('PreviewServer');
+const logger = new Logger('PreviewServer');
 
 // Classes
 export class PreviewServerFactory {

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/helper.ts

@@ -11,7 +11,7 @@ import {
   AIO_NGINX_PORT_HTTPS,
   AIO_WWW_USER,
 } from '../common/env-variables';
-import {computeShortSha, createLogger} from '../common/utils';
+import {computeShortSha, Logger} from '../common/utils';
 
 // Interfaces - Types
 export interface CmdResult { success: boolean; err: Error | null; stdout: string; stderr: string; }
@@ -31,7 +31,7 @@ class Helper {
     https: AIO_NGINX_PORT_HTTPS,
   };
 
-  private logger = createLogger('TestHelper');
+  private logger = new Logger('TestHelper');
 
   // Constructor
   constructor() {

aio/aio-builds-setup/dockerbuild/scripts-js/lib/verify-setup/mock-external-apis.ts

@@ -2,7 +2,7 @@
 import * as nock from 'nock';
 import * as tar from 'tar-stream';
 import {gzipSync} from 'zlib';
-import {createLogger, getEnvVar} from '../common/utils';
+import {getEnvVar, Logger} from '../common/utils';
 import {BuildNums, PrNums, SHA} from './constants';
 
 // We are using the `nock` library to fake responses from REST requests, when testing.
@@ -14,7 +14,7 @@ import {BuildNums, PrNums, SHA} from './constants';
 // below and return a suitable response. This is quite complicated to setup since the
 // response from, say, CircleCI will affect what request is made to, say, Github.
 
-const logger = createLogger('NOCK');
+const logger = new Logger('mock-external-apis');
 
 const log = (...args: any[]) => {
   // Filter out non-matching URL checks

aio/aio-builds-setup/dockerbuild/scripts-js/test/clean-up/build-cleaner.spec.ts

@@ -5,6 +5,7 @@ import * as shell from 'shelljs';
 import {BuildCleaner} from '../../lib/clean-up/build-cleaner';
 import {HIDDEN_DIR_PREFIX} from '../../lib/common/constants';
 import {GithubPullRequests} from '../../lib/common/github-pull-requests';
+import {Logger} from '../../lib/common/utils';
 
 const EXISTING_BUILDS = [10, 20, 30, 40];
 const EXISTING_DOWNLOADS = [
@@ -18,11 +19,13 @@ const ANY_DATE = jasmine.any(String);
 
 // Tests
 describe('BuildCleaner', () => {
+  let loggerErrorSpy: jasmine.Spy;
+  let loggerLogSpy: jasmine.Spy;
   let cleaner: BuildCleaner;
 
   beforeEach(() => {
-    spyOn(console, 'error');
-    spyOn(console, 'log');
+    loggerErrorSpy = spyOn(Logger.prototype, 'error');
+    loggerLogSpy = spyOn(Logger.prototype, 'log');
     cleaner = new BuildCleaner('/foo/bar', 'baz', 'qux', '12345', 'downloads', 'build.zip');
   });
 
@@ -279,7 +282,8 @@ describe('BuildCleaner', () => {
 
     it('should log the number of open PRs', () => {
       promise.then(prNumbers => {
-        expect(console.log).toHaveBeenCalledWith(ANY_DATE, 'BuildCleaner:        ', `Open pull requests: ${prNumbers}`);
+        expect(loggerLogSpy).toHaveBeenCalledWith(
+          ANY_DATE, 'BuildCleaner:        ', `Open pull requests: ${prNumbers}`);
       });
     });
   });
@@ -383,8 +387,7 @@ describe('BuildCleaner', () => {
 
       cleaner.removeDir('/foo/bar');
 
-      expect(console.error).toHaveBeenCalledWith(
-        jasmine.any(String), 'BuildCleaner:        ', 'ERROR: Unable to remove \'/foo/bar\' due to:', 'Test');
+      expect(loggerErrorSpy).toHaveBeenCalledWith('ERROR: Unable to remove \'/foo/bar\' due to:', 'Test');
     });
 
   });
@@ -401,8 +404,8 @@ describe('BuildCleaner', () => {
     it('should log the number of existing builds and builds to be removed', () => {
       cleaner.removeUnnecessaryBuilds([1, 2, 3], [3, 4, 5, 6]);
 
-      expect(console.log).toHaveBeenCalledWith(ANY_DATE, 'BuildCleaner:        ', 'Existing builds: 3');
-      expect(console.log).toHaveBeenCalledWith(ANY_DATE, 'BuildCleaner:        ', 'Removing 2 build(s): 1, 2');
+      expect(loggerLogSpy).toHaveBeenCalledWith('Existing builds: 3');
+      expect(loggerLogSpy).toHaveBeenCalledWith('Removing 2 build(s): 1, 2');
     });
 
 
@@ -470,8 +473,8 @@ describe('BuildCleaner', () => {
     it('should log the number of existing builds and builds to be removed', () => {
       cleaner.removeUnnecessaryDownloads(EXISTING_DOWNLOADS, OPEN_PRS);
 
-      expect(console.log).toHaveBeenCalledWith(ANY_DATE, 'BuildCleaner:        ', 'Existing downloads: 4');
-      expect(console.log).toHaveBeenCalledWith(ANY_DATE, 'BuildCleaner:        ',
+      expect(loggerLogSpy).toHaveBeenCalledWith('Existing downloads: 4');
+      expect(loggerLogSpy).toHaveBeenCalledWith(
         'Removing 2 download(s): downloads/20-ABCDEF0-build.zip, downloads/20-1234567-build.zip');
     });
   });

aio/aio-builds-setup/dockerbuild/scripts-js/test/common/github-pull-requests.spec.ts

@@ -101,10 +101,7 @@ describe('GithubPullRequests', () => {
   describe('fetchAll()', () => {
     let prs: GithubPullRequests;
 
-    beforeEach(() => {
-      prs = new GithubPullRequests(githubApi, 'foo', 'bar');
-      spyOn(console, 'log');
-    });
+    beforeEach(() => prs = new GithubPullRequests(githubApi, 'foo', 'bar'));
 
 
     it('should call \'getPaginated()\' with the correct pathname and params', () => {

aio/aio-builds-setup/dockerbuild/scripts-js/test/common/utils.spec.ts

@@ -6,6 +6,7 @@ import {
   computeShortSha,
   getEnvVar,
   getPrInfoFromDownloadPath,
+  Logger,
 } from '../../lib/common/utils';
 
 // Tests
@@ -19,6 +20,7 @@ describe('utils', () => {
     });
   });
 
+
   describe('assert', () => {
     it('should throw if passed a false value', () => {
       expect(() => assert(false, 'error message')).toThrowError('error message');
@@ -29,6 +31,7 @@ describe('utils', () => {
     });
   });
 
+
   describe('computeArtifactDownloadPath', () => {
     it('should compute an absolute path based on the artifact info provided', () => {
       const downloadDir = '/a/b/c';
@@ -40,6 +43,7 @@ describe('utils', () => {
     });
   });
 
+
   describe('getPrInfoFromDownloadPath', () => {
     it('should extract the PR and SHA from the file path', () => {
       const {pr, sha} = getPrInfoFromDownloadPath('a/b/c/12345-ABCDE-artifact.zip');
@@ -48,6 +52,7 @@ describe('utils', () => {
     });
   });
 
+
   describe('assertNotMissingOrEmpty()', () => {
 
     it('should throw if passed an empty value', () => {
@@ -122,4 +127,79 @@ describe('utils', () => {
 
   });
 
+
+  describe('Logger', () => {
+    let consoleErrorSpy: jasmine.Spy;
+    let consoleInfoSpy: jasmine.Spy;
+    let consoleLogSpy: jasmine.Spy;
+    let consoleWarnSpy: jasmine.Spy;
+    let logger: Logger;
+
+    beforeEach(() => {
+      consoleErrorSpy = spyOn(console, 'error');
+      consoleInfoSpy = spyOn(console, 'info');
+      consoleLogSpy = spyOn(console, 'log');
+      consoleWarnSpy = spyOn(console, 'warn');
+
+      logger = new Logger('TestScope');
+    });
+
+
+    it('should delegate to `console`', () => {
+      logger.error('foo');
+      expect(consoleErrorSpy).toHaveBeenCalledTimes(1);
+      expect(consoleErrorSpy.calls.argsFor(0)).toContain('foo');
+
+      logger.info('bar');
+      expect(consoleInfoSpy).toHaveBeenCalledTimes(1);
+      expect(consoleInfoSpy.calls.argsFor(0)).toContain('bar');
+
+      logger.log('baz');
+      expect(consoleLogSpy).toHaveBeenCalledTimes(1);
+      expect(consoleLogSpy.calls.argsFor(0)).toContain('baz');
+
+      logger.warn('qux');
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1);
+      expect(consoleWarnSpy.calls.argsFor(0)).toContain('qux');
+    });
+
+
+    it('should prepend messages with the current date and logger\'s scope', () => {
+      const mockDate = new Date(1337);
+      const expectedDateStr = `[${mockDate}]`;
+      const expectedScopeStr = 'TestScope:           ';
+
+      jasmine.clock().mockDate(mockDate);
+      jasmine.clock().withMock(() => {
+        logger.error();
+        logger.info();
+        logger.log();
+        logger.warn();
+      });
+
+      expect(consoleErrorSpy).toHaveBeenCalledWith(expectedDateStr, expectedScopeStr);
+      expect(consoleInfoSpy).toHaveBeenCalledWith(expectedDateStr, expectedScopeStr);
+      expect(consoleLogSpy).toHaveBeenCalledWith(expectedDateStr, expectedScopeStr);
+      expect(consoleWarnSpy).toHaveBeenCalledWith(expectedDateStr, expectedScopeStr);
+    });
+
+
+    it('should pass all arguments to `console`', () => {
+      const someString = jasmine.any(String);
+
+      logger.error('foo1', 'foo2');
+      expect(consoleErrorSpy).toHaveBeenCalledWith(someString, someString, 'foo1', 'foo2');
+
+      logger.info('bar1', 'bar2');
+      expect(consoleInfoSpy).toHaveBeenCalledWith(someString, someString, 'bar1', 'bar2');
+
+      logger.log('baz1', 'baz2');
+      expect(consoleLogSpy).toHaveBeenCalledWith(someString, someString, 'baz1', 'baz2');
+
+      logger.warn('qux1', 'qux2');
+      expect(consoleWarnSpy).toHaveBeenCalledWith(someString, someString, 'qux1', 'qux2');
+    });
+
+  });
+
 });

aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/build-creator.spec.ts

@@ -5,6 +5,7 @@ import * as fs from 'fs';
 import * as path from 'path';
 import * as shell from 'shelljs';
 import {SHORT_SHA_LEN} from '../../lib/common/constants';
+import {Logger} from '../../lib/common/utils';
 import {BuildCreator} from '../../lib/preview-server/build-creator';
 import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/preview-server/build-events';
 import {PreviewServerError} from '../../lib/preview-server/preview-error';
@@ -491,7 +492,7 @@ describe('BuildCreator', () => {
     beforeEach(() => {
       cpExecCbs = [];
 
-      consoleWarnSpy = spyOn(console, 'warn');
+      consoleWarnSpy = spyOn(Logger.prototype, 'warn');
       shellChmodSpy = spyOn(shell, 'chmod');
       shellRmSpy = spyOn(shell, 'rm');
       cpExecSpy = spyOn(cp, 'exec').and.callFake((_: string, cb: (...args: any[]) => void) => cpExecCbs.push(cb));
@@ -513,8 +514,7 @@ describe('BuildCreator', () => {
 
     it('should log (as a warning) any stderr output if extracting succeeded', done => {
       (bc as any).extractArchive('foo', 'bar').
-        then(() => expect(consoleWarnSpy)
-          .toHaveBeenCalledWith(jasmine.any(String), 'BuildCreator:        ', 'This is stderr')).
+        then(() => expect(consoleWarnSpy).toHaveBeenCalledWith('This is stderr')).
         then(done);
 
       cpExecCbs[0](null, 'This is stdout', 'This is stderr');

aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/build-retriever.spec.ts

@@ -1,6 +1,7 @@
 import * as fs from 'fs';
 import * as nock from 'nock';
 import {BuildInfo, CircleCiApi} from '../../lib/common/circle-ci-api';
+import {Logger} from '../../lib/common/utils';
 import {BuildRetriever} from '../../lib/preview-server/build-retriever';
 
 describe('BuildRetriever', () => {
@@ -29,10 +30,6 @@ describe('BuildRetriever', () => {
       vcs_revision: 'COMMIT',
     };
 
-    spyOn(console, 'log');
-    spyOn(console, 'warn');
-    spyOn(console, 'error');
-
     api = new CircleCiApi('ORG', 'REPO', 'TOKEN');
     spyOn(api, 'getBuildInfo').and.callFake(() => Promise.resolve(BUILD_INFO));
     getBuildArtifactUrlSpy = spyOn(api, 'getBuildArtifactUrl')
@@ -91,6 +88,7 @@ describe('BuildRetriever', () => {
     let retriever: BuildRetriever;
 
     beforeEach(() => {
+      spyOn(Logger.prototype, 'warn');
       retriever = new BuildRetriever(api, MAX_DOWNLOAD_SIZE, DOWNLOAD_DIR);
     });
 

aio/aio-builds-setup/dockerbuild/scripts-js/test/preview-server/preview-server-factory.spec.ts

@@ -7,6 +7,7 @@ import {CircleCiApi} from '../../lib/common/circle-ci-api';
 import {GithubApi} from '../../lib/common/github-api';
 import {GithubPullRequests} from '../../lib/common/github-pull-requests';
 import {GithubTeams} from '../../lib/common/github-teams';
+import {Logger} from '../../lib/common/utils';
 import {BuildCreator} from '../../lib/preview-server/build-creator';
 import {ChangedPrVisibilityEvent, CreatedBuildEvent} from '../../lib/preview-server/build-events';
 import {BuildRetriever, GithubInfo} from '../../lib/preview-server/build-retriever';
@@ -38,15 +39,17 @@ describe('PreviewServerFactory', () => {
     significantFilesPattern: '^(?:aio|packages)\\/(?!.*[._]spec\\.[jt]s$)',
     trustedPrLabel: 'trusted: pr-label',
   };
+  let loggerInfoSpy: jasmine.Spy;
+  let loggerLogSpy: jasmine.Spy;
 
   // Helpers
   const createPreviewServer = (partialConfig: Partial<PreviewServerConfig> = {}) =>
     PreviewServerFactory.create({...defaultConfig, ...partialConfig});
 
   beforeEach(() => {
-    spyOn(console, 'error');
-    spyOn(console, 'info');
-    spyOn(console, 'log');
+    loggerInfoSpy = spyOn(Logger.prototype, 'info');
+    loggerLogSpy = spyOn(Logger.prototype, 'log');
+    spyOn(Logger.prototype, 'error');
   });
 
   describe('create()', () => {
@@ -140,11 +143,10 @@ describe('PreviewServerFactory', () => {
       const server = createPreviewServer();
       server.address = () => ({address: 'foo', family: '', port: 1337});
 
-      expect(console.info).not.toHaveBeenCalled();
+      expect(loggerInfoSpy).not.toHaveBeenCalled();
 
       server.emit('listening');
-      expect(console.info).toHaveBeenCalledWith(
-        jasmine.any(String), 'PreviewServer:       ', 'Up and running (and listening on foo:1337)...');
+      expect(loggerInfoSpy).toHaveBeenCalledWith('Up and running (and listening on foo:1337)...');
     });
 
   });
@@ -359,8 +361,8 @@ describe('PreviewServerFactory', () => {
         await agent.post(URL).send(BASIC_PAYLOAD).expect(204);
         expect(getGithubInfoSpy).not.toHaveBeenCalled();
         expect(getSignificantFilesChangedSpy).not.toHaveBeenCalled();
-        expect(console.log).toHaveBeenCalledWith(jasmine.any(String), 'PreviewServer:       ',
-        'Build:12345, Job:lint -', 'Skipping preview processing because this is not the "aio_preview" job.');
+        expect(loggerLogSpy).toHaveBeenCalledWith(
+          'Build:12345, Job:lint -', 'Skipping preview processing because this is not the "aio_preview" job.');
         expect(downloadBuildArtifactSpy).not.toHaveBeenCalled();
         expect(getPrIsTrustedSpy).not.toHaveBeenCalled();
         expect(createBuildSpy).not.toHaveBeenCalled();
@@ -371,7 +373,7 @@ describe('PreviewServerFactory', () => {
         await agent.post(URL).send(BASIC_PAYLOAD).expect(204);
         expect(getGithubInfoSpy).toHaveBeenCalledWith(BUILD_NUM);
         expect(getSignificantFilesChangedSpy).toHaveBeenCalledWith(PR, jasmine.any(RegExp));
-        expect(console.log).toHaveBeenCalledWith(jasmine.any(String), 'PreviewServer:       ',
+        expect(loggerLogSpy).toHaveBeenCalledWith(
           'PR:777, Build:12345 - Skipping preview processing because this PR did not touch any significant files.');
         expect(downloadBuildArtifactSpy).not.toHaveBeenCalled();
         expect(getPrIsTrustedSpy).not.toHaveBeenCalled();