Merge pull request #97 from appium/isaac-logger

Remove logger, and start using one from appium-support

README.md

@@ -15,7 +15,6 @@ This is the parent class that all [appium](appium.io) drivers inherit from, alon
 * [The Mobile JSON Wire Protocol Encapsulation](lib/mjsonwp)
 * [The JSONWP Proxy Library](lib/jsonwp-proxy)
 * [The JSONWP Status Library](lib/jsonwp-status)
-* [The Appium Logger](lib/logger)
 
 *Note*: Issue tracking for this repo has been disabled. Please use the [main Appium issue tracker](https://github.com/appium/appium/issues) instead.
 

index.js

@@ -27,7 +27,3 @@ export { JWProxy };
 import { codes, getSummaryByCode } from './lib/jsonwp-status/status';
 const statusCodes = codes;
 export { statusCodes, getSummaryByCode };
-
-// logger exports
-import { getLogger, patchLogger } from './lib/logger/logger';
-export { getLogger, patchLogger };

lib/basedriver/logger.js

@@ -1,4 +1,4 @@
-import { getLogger } from '../logger/logger';
+import { logger } from 'appium-support';
 
-const log = getLogger('BaseDriver');
+const log = logger.getLogger('BaseDriver');
 export default log;

lib/express/logger.js

@@ -1,4 +1,4 @@
-import { getLogger } from '../logger/logger';
+import { logger } from 'appium-support';
 
-const log = getLogger('HTTP');
+const log = logger.getLogger('HTTP');
 export default log;

lib/jsonwp-proxy/proxy.js

@@ -1,13 +1,12 @@
 // jshint ignore: start
 import _ from 'lodash';
-import { getLogger } from '../logger/logger';
-import { util } from 'appium-support';
+import { logger, util } from 'appium-support';
 import request from 'request-promise';
 import { getSummaryByCode } from '../jsonwp-status/status';
 import { errors } from '../mjsonwp/errors';
 
 
-const log = getLogger('JSONWP Proxy');
+const log = logger.getLogger('JSONWP Proxy');
 // TODO: Make this value configurable as a server side capability
 const LOG_OBJ_LENGTH = 1024; // MAX LENGTH Logged to file / console
 

lib/logger/README.md

@@ -1,89 +0,0 @@
-## logger
-
-Basic logger defaulting to [npmlog](https://github.com/npm/npmlog) with special consideration for running
-tests (doesn't output logs when run with `_TESTING=1` in the env).
-
-### Logging levels
-
-There are a number of levels, exposed as methods on the log object, at which logging can be made. The built-in ones correspond to those of [npmlog](https://github.com/npm/npmlog#loglevelprefix-message-), and are:
-`silly`, `verbose`, `info`, `http`, `warn`, and `error`. In addition there is a `debug` level.
-
-The default threshhold level is `verbose`.
-
-The logged output, by default, will be `level prefix message`. So
-
-```js
-import { getLogger } from 'appium-base-driver';
-let log = getLogger('mymodule');
-log.warn('a warning');`
-```
-
-Will produce
-
-```shell
-warn mymodule a warning
-```
-
-
-### Environment variables
-
-There are two environment variable flags that affect the way `appium-base-driver` `logger` works.
-
-`_TESTING`
-
-- `_TESTING=1` stops output of logs when set to `1`.
-
-`_FORCE_LOGS`
-
-- This flag, when set to `1`, reverses the `_TESTING`
-
-
-### Usage
-
-`log.level`
-
-- get and set the threshhold level at which to display the logs. Any logs at or above this level will be displayed. The special level silent will prevent anything from being displayed ever. See [npmlog#level](https://github.com/npm/npmlog#loglevel).
-
-`log[level](message)`
-
-- logs to `level`
-    ```js
-    import { getLogger } from 'appium-base-driver';
-    let log = getLogger('mymodule');
-
-    log.info('hi!');
-    // => info mymodule hi!
-    ```
-
-`log.unwrap()`
-
-- retrieves the underlying [npmlog](https://github.com/npm/npmlog) object, in order to manage how logging is done at a low level (e.g., changing output streams, retrieving an array of messages, adding log levels, etc.).
-
-    ```js
-    import { getLogger } from 'appium-base-driver';
-    let log = getLogger('mymodule');
-
-    log.info('hi!');
-
-    let npmlogger = log.unwrap();
-
-    // any `npmlog` methods
-    let logs = npmlogger.record;
-    // logs === [ { id: 0, level: 'info', prefix: 'mymodule', message: 'hi!', messageRaw: [ 'hi!' ] }]
-    ```
-
-`log.errorAndThrow(error)`
-
-- logs the error passed in, at `error` level, and then throws the error. If the error passed in is not an instance of [Error](https://nodejs.org/api/errors.html#errors_class_error) (either directly, or a subclass of `Error`) it will be wrapped in a generic `Error` object.
-
-    ```js
-    import { getLogger } from 'appium-base-driver';
-    let log = getLogger('mymodule');
-
-    // previously there would be two lines
-    log.error('This is an error');
-    throw new Error('This is an error');
-
-    // now is compacted
-    log.errorAndThrow('This is an error');
-    ```

lib/logger/logger.js

@@ -1,80 +0,0 @@
-import npmlog from 'npmlog';
-
-
-// levels that are available from `npmlog`
-const NPM_LEVELS = ['silly', 'verbose', 'debug', 'info', 'http', 'warn', 'error'];
-
-// mock log object used in testing mode
-let mockLog = {};
-for (let level of NPM_LEVELS) {
-  mockLog[level] = () => {};
-}
-
-function patchLogger (logger) {
-  if (!logger.debug) {
-    logger.addLevel('debug', 1000, { fg: 'blue', bg: 'black' }, 'dbug');
-  }
-}
-
-function _getLogger () {
-  // check if the user set the `_TESTING` or `_FORCE_LOGS` flag
-  const testingMode = parseInt(process.env._TESTING, 10) === 1;
-  const forceLogMode = parseInt(process.env._FORCE_LOGS, 10) === 1;
-
-  // if is possible that there is a logger instance that is already around,
-  // in which case we want t o use that
-  const usingGlobalLog = !!global._global_npmlog;
-  let logger;
-  if (testingMode && !forceLogMode) {
-    // in testing mode, use a mock logger object that we can query
-    logger = mockLog;
-  } else {
-    // otherwise, either use the global, or a new `npmlog` object
-    logger = global._global_npmlog || npmlog;
-  }
-  patchLogger(logger);
-  return [logger, usingGlobalLog];
-}
-
-function getLogger (prefix = null) {
-  let [logger, usingGlobalLog] = _getLogger();
-
-  // wrap the logger so that we can catch and modify any logging
-  let wrappedLogger = {unwrap: () => logger};
-
-  // allow access to the level of the underlying logger
-  Object.defineProperty(wrappedLogger, 'level', {
-    get: () => { return logger.level; },
-    set: (newValue) => { logger.level = newValue; },
-    enumerable: true,
-    configurable: true
-  });
-  // add all the levels from `npmlog`, and map to the underlying logger
-  for (let level of NPM_LEVELS) {
-    wrappedLogger[level] = logger[level].bind(logger, prefix);
-  }
-  // add method to log an error, and throw it, for convenience
-  wrappedLogger.errorAndThrow = function (err) {
-    // make sure we have an `Error` object. Wrap if necessary
-    if (!(err instanceof Error)) {
-      err = new Error(err);
-    }
-    // log and throw
-    this.error(err);
-    throw err;
-  };
-  if (!usingGlobalLog) {
-    // if we're not using a global log specified from some top-level package,
-    // set the log level to a default of verbose. Otherwise, let the top-level
-    // package set the log level
-    wrappedLogger.level = 'verbose';
-  }
-  wrappedLogger.levels = NPM_LEVELS;
-  return wrappedLogger;
-}
-
-// export a default logger with no prefix
-const log = getLogger();
-
-export { patchLogger, getLogger };
-export default log;

lib/mjsonwp/mjsonwp.js

@@ -1,13 +1,13 @@
 // jshint ignore: start
 import _ from 'lodash';
-import { getLogger } from '../logger/logger';
+import { logger, util } from 'appium-support';
 import { validators } from './validators';
 import { errors, isErrorType, MJSONWPError, errorFromCode } from './errors';
 import { METHOD_MAP, NO_SESSION_ID_COMMANDS } from './routes';
 import B from 'bluebird';
-import { util } from 'appium-support';
 
-const log = getLogger('MJSONWP');
+
+const log = logger.getLogger('MJSONWP');
 const JSONWP_SUCCESS_STATUS_CODE = 0;
 // TODO: Make this value configurable as a server side capability
 const LOG_OBJ_LENGTH = 1024; // MAX LENGTH Logged to file / console

package.json

@@ -24,7 +24,7 @@
   },
   "dependencies": {
     "adm-zip": "^0.4.7",
-    "appium-support": "^2.0.9",
+    "appium-support": "^2.4.0",
     "asyncbox": "^2.3.1",
     "babel-runtime": "=5.8.24",
     "bluebird": "^2.10.2",
@@ -35,7 +35,6 @@
     "lodash": "^4.0.0",
     "method-override": "^2.3.5",
     "morgan": "^1.6.1",
-    "npmlog": "^4.0.1",
     "request": "^2.64.0",
     "request-promise": "^4.1.1",
     "serve-favicon": "^2.3.0",

test/logger/helpers.js

@@ -1,54 +0,0 @@
-import chai from 'chai';
-import sinon from 'sinon';
-import _ from 'lodash';
-import { getLogger } from '../..';
-
-
-chai.should();
-
-function setupWriters () {
-  return {'stdout': sinon.spy(process.stdout, 'write'),
-          'stderr': sinon.spy(process.stderr, 'write')};
-}
-
-function getDynamicLogger (testingMode, forceLogs, prefix=null) {
-  process.env._TESTING = testingMode ? '1' : '0';
-  process.env._FORCE_LOGS = forceLogs ? '1' : '0';
-  return getLogger(prefix);
-}
-
-function restoreWriters (writers) {
-  for (let w of _.values(writers)) {
-    w.restore();
-  }
-}
-
-function someoneHadOutput (writers, output) {
-  let hadOutput = false;
-  let matchOutput = sinon.match(function (value) {
-    return value && value.indexOf(output) >= 0;
-  }, 'matchOutput');
-
-  for (let writer of _.values(writers)) {
-    if (writer.calledWith) {
-      hadOutput = writer.calledWithMatch(matchOutput);
-      if (hadOutput) break;
-    }
-  }
-  return hadOutput;
-}
-
-function assertOutputContains (writers, output) {
-  if (!someoneHadOutput(writers, output)) {
-    throw new Error(`Expected something to have been called with: '${output}'`);
-  }
-}
-
-function assertOutputDoesntContain (writers, output) {
-  if (someoneHadOutput(writers, output)) {
-    throw new Error(`Expected nothing to have been called with: '${output}'`);
-  }
-}
-
-export { setupWriters, restoreWriters, assertOutputContains,
-         assertOutputDoesntContain, getDynamicLogger };

test/logger/logger-force-specs.js

@@ -1,37 +0,0 @@
-// transpile:mocha
-
-import { getDynamicLogger, restoreWriters, setupWriters,
-         assertOutputContains } from './helpers';
-
-describe('logger with force log', () => {
-  let writers, log;
-  before(() => {
-    writers = setupWriters();
-    log = getDynamicLogger(true, true);
-    log.level = 'silly';
-  });
-
-  after(() => {
-    restoreWriters(writers);
-  });
-
-  it('should not rewrite log levels even during testing', () => {
-    log.silly('silly');
-    assertOutputContains(writers, 'silly');
-    log.verbose('verbose');
-    assertOutputContains(writers, 'verbose');
-    log.verbose('debug');
-    assertOutputContains(writers, 'debug');
-    log.info('info');
-    assertOutputContains(writers, 'info');
-    log.http('http');
-    assertOutputContains(writers, 'http');
-    log.warn('warn');
-    assertOutputContains(writers, 'warn');
-    log.error('error');
-    assertOutputContains(writers, 'error');
-    (() => { log.errorAndThrow('msg'); }).should.throw('msg');
-    assertOutputContains(writers, 'error');
-    assertOutputContains(writers, 'msg');
-  });
-});

test/logger/logger-normal-specs.js

@@ -1,90 +0,0 @@
-// transpile:mocha
-
-import { getDynamicLogger, restoreWriters, setupWriters,
-         assertOutputContains, assertOutputDoesntContain } from './helpers';
-
-describe('normal logger', () => {
-  let writers, log;
-  beforeEach(() => {
-    writers = setupWriters();
-    log = getDynamicLogger(false, false);
-    log.level = 'silly';
-  });
-
-  afterEach(() => {
-    restoreWriters(writers);
-  });
-
-  it('should not rewrite log levels outside of testing', () => {
-    log.silly('silly');
-    assertOutputContains(writers, 'silly');
-    log.verbose('verbose');
-    assertOutputContains(writers, 'verbose');
-    log.verbose('debug');
-    assertOutputContains(writers, 'debug');
-    log.info('info');
-    assertOutputContains(writers, 'info');
-    log.http('http');
-    assertOutputContains(writers, 'http');
-    log.warn('warn');
-    assertOutputContains(writers, 'warn');
-    log.error('error');
-    assertOutputContains(writers, 'error');
-  });
-  it('throw should not rewrite log levels outside of testing and throw error', () => {
-    (() => { log.errorAndThrow('msg1'); }).should.throw('msg1');
-    (() => { log.errorAndThrow(new Error('msg2')); }).should.throw('msg2');
-    assertOutputContains(writers, 'msg1');
-    assertOutputContains(writers, 'msg2');
-  });
-  it('should get and set log levels', () => {
-    log.level = 'warn';
-    log.level.should.equal('warn');
-    log.info('information');
-    log.warn('warning');
-    assertOutputDoesntContain(writers, 'information');
-    assertOutputContains(writers, 'warning');
-  });
-});
-
-describe('normal logger with prefix', () => {
-  let writers, log;
-  before(() => {
-    writers = setupWriters();
-    log = getDynamicLogger(false, false, 'myprefix');
-    log.level = 'silly';
-  });
-
-  after(() => {
-    restoreWriters(writers);
-  });
-
-  it('should not rewrite log levels outside of testing', () => {
-    log.silly('silly');
-    assertOutputContains(writers, 'silly');
-    assertOutputContains(writers, 'myprefix');
-    log.verbose('verbose');
-    assertOutputContains(writers, 'verbose');
-    assertOutputContains(writers, 'myprefix');
-    log.verbose('debug');
-    assertOutputContains(writers, 'debug');
-    assertOutputContains(writers, 'myprefix');
-    log.info('info');
-    assertOutputContains(writers, 'info');
-    assertOutputContains(writers, 'myprefix');
-    log.http('http');
-    assertOutputContains(writers, 'http');
-    assertOutputContains(writers, 'myprefix');
-    log.warn('warn');
-    assertOutputContains(writers, 'warn');
-    assertOutputContains(writers, 'myprefix');
-    log.error('error');
-    assertOutputContains(writers, 'error');
-    assertOutputContains(writers, 'myprefix');
-  });
-  it('throw should not rewrite log levels outside of testing and throw error', () => {
-    (() => { log.errorAndThrow('msg'); }).should.throw('msg');
-    assertOutputContains(writers, 'error');
-    assertOutputContains(writers, 'myprefix');
-  });
-});

test/logger/logger-test-specs.js

@@ -1,38 +0,0 @@
-// transpile:mocha
-
-import { getDynamicLogger, restoreWriters, setupWriters,
-         assertOutputDoesntContain } from './helpers';
-
-describe('test logger', () => {
-  let writers, log;
-  before(() => {
-    writers = setupWriters();
-    log = getDynamicLogger(true);
-  });
-
-  after(() => {
-    restoreWriters(writers);
-  });
-
-  it('should contains levels', () => {
-    log.levels.should.have.length.above(3);
-    log.levels[2].should.equal('debug');
-  });
-
-  it('should unwrap', () => {
-    log.unwrap.should.exist;
-    log.unwrap().should.exist;
-  });
-
-  it('should rewrite npmlog levels during testing', () => {
-    const text = 'hi';
-    log.silly(text);
-    log.verbose(text);
-    log.info(text);
-    log.http(text);
-    log.warn(text);
-    log.error(text);
-    (() => { log.errorAndThrow(text); }).should.throw(text);
-    assertOutputDoesntContain(writers, text);
-  });
-});