Instrument Repo for Code Coverage (#268)

* First pass coverage implementation

* Instrumenting node coverage where appropriate

* [AUTOMATED]: Prettier Code Styling

* Configure to pipe to coveralls

* Configure karma to report coverage by browser

* Augment .travis.yml to report coverage

* Refacor .travis.yml to use yarn for consistency

* Refactor to properly use lcovonly reporter

* Refactor to run coverage scripts only on success

* [AUTOMATED]: Prettier Code Styling

Author: jshcrowthe

.gitignore

@@ -14,3 +14,7 @@ dist
 .idea
 .vscode
 *.iml
+
+# Coverage
+.nyc_output
+coverage

.travis.yml

@@ -14,7 +14,9 @@ before_install:
 before_script:
   - cp config/ci.config.json config/project.json
 script:
-  - xvfb-run npm test
+  - xvfb-run yarn test
+after_success:
+  - yarn test:coverage
 
 # Misc Addons/Configs
 dist: trusty

config/karma.base.js

@@ -66,7 +66,7 @@ const config = {
   // test results reporter to use
   // possible values: 'dots', 'progress'
   // available reporters: https://npmjs.org/browse/keyword/karma-reporter
-  reporters: ['spec' /*, 'saucelabs' */],
+  reporters: ['spec', 'coverage-istanbul' /*, 'saucelabs' */],
 
   // web server port
   port: 8089,
@@ -113,6 +113,12 @@ const config = {
 
     // Pass through --grep option to filter the tests that run.
     args: argv.grep ? ['--grep', argv.grep] : []
+  },
+
+  coverageIstanbulReporter: {
+    dir: path.resolve(process.cwd(), 'coverage/browser/%browser%'),
+    fixWebpackSourcePaths: true,
+    reports: ['lcovonly']
   }
 };
 

config/webpack.test.js

@@ -18,7 +18,7 @@ const path = require('path');
 const webpack = require('webpack');
 
 module.exports = {
-  devtool: 'eval-source-map',
+  devtool: 'source-map',
   module: {
     rules: [
       {
@@ -27,9 +27,18 @@ module.exports = {
         use: 'ts-loader'
       },
       {
-        test: /\.js$/,
-        use: ['source-map-loader'],
+        test: /\.[tj]sx?$/,
+        use: 'source-map-loader',
         enforce: 'pre'
+      },
+      {
+        test: /\.tsx?$/,
+        use: {
+          loader: 'istanbul-instrumenter-loader',
+          options: { esModules: true }
+        },
+        enforce: 'post',
+        include: path.resolve(process.cwd(), 'src')
       }
     ]
   },

integration/firestore/gulpfile.js

@@ -36,9 +36,9 @@ function reworkFirebasePaths() {
         /**
          * This regex is designed to match the following statement used in our
          * firestore integratino test suites:
-         * 
+         *
          * import firebase from '../../util/firebase_export';
-         *  
+         *
          * It will handle variations in whitespace, single/double quote
          * differences, as well as different paths to a valid firebase_export
          */

integration/shared/validator.js

@@ -43,7 +43,7 @@ function validateNamespace(definition, candidate) {
     const candidateChunk = candidate[key];
 
     /**
-     * Grab all of the keys that aren't meta properties and capture 
+     * Grab all of the keys that aren't meta properties and capture
      * them for more testing later
      */
     const internalKeys = Object.keys(definitionChunk).filter(
@@ -72,16 +72,16 @@ function validateNamespace(definition, candidate) {
       }
 
       /**
-       * Keys marked with `__return` allow us to validate the 
+       * Keys marked with `__return` allow us to validate the
        * return value of a specific part of the API
-       * 
+       *
        * e.g.
        * {
        *   ...
        *   app: {
        *     __return: {
        *       <PROPERTIES OF AN APP>
-       *     } 
+       *     }
        *   }
        * }
        */

package.json

@@ -23,6 +23,8 @@
     "repl": "node tools/repl.js",
     "pretest": "node tools/pretest.js",
     "test": "lerna run --parallel test",
+    "pretest:coverage": "mkdirp coverage",
+    "test:coverage": "lcov-result-merger 'packages/**/lcov.info' | coveralls",
     "test:setup": "node tools/config.js"
   },
   "repository": {
@@ -36,14 +38,18 @@
   "devDependencies": {
     "chalk": "^2.3.0",
     "child-process-promise": "^2.2.1",
+    "coveralls": "^3.0.0",
     "firebase-tools": "^3.10.1",
     "glob": "^7.1.2",
     "gulp-sourcemaps": "^2.6.1",
     "gulp-typescript": "^3.2.3",
     "husky": "^0.14.3",
     "inquirer": "^3.2.3",
+    "istanbul-instrumenter-loader": "^3.0.0",
+    "lcov-result-merger": "^1.2.0",
     "lerna": "^2.1.0",
     "merge2": "^1.2.0",
+    "mkdirp": "^0.5.1",
     "mz": "^2.7.0",
     "ora": "^1.3.0",
     "prettier": "^1.7.0",

packages/app/package.json

@@ -8,7 +8,8 @@
     "dev": "gulp dev",
     "test": "run-p test:browser test:node",
     "test:browser": "karma start --single-run",
-    "test:node": "mocha test/**/*.test.* --compilers ts:ts-node/register --exit",
+    "test:browser:debug": "karma start --browsers Chrome --auto-watch",
+    "test:node": "nyc --reporter lcovonly -- mocha test/**/*.test.* --compilers ts:ts-node/register --exit",
     "prepare": "gulp build"
   },
   "license": "Apache-2.0",
@@ -26,13 +27,15 @@
     "karma": "^1.7.0",
     "karma-chrome-launcher": "^2.2.0",
     "karma-cli": "^1.0.1",
+    "karma-coverage-istanbul-reporter": "^1.3.0",
     "karma-mocha": "^1.3.0",
     "karma-sauce-launcher": "^1.2.0",
     "karma-sourcemap-loader": "^0.3.7",
     "karma-spec-reporter": "^0.0.31",
     "karma-webpack": "^2.0.4",
     "mocha": "^4.0.1",
     "npm-run-all": "^4.1.1",
+    "nyc": "^11.2.1",
     "sinon": "^4.0.2",
     "source-map-loader": "^0.2.3",
     "ts-loader": "^3.1.0",
@@ -47,5 +50,11 @@
   "bugs": {
     "url": "https://github.com/firebase/firebase-js-sdk/issues"
   },
-  "typings": "dist/index.d.ts"
+  "typings": "dist/index.d.ts",
+  "nyc": {
+    "extension": [
+      ".ts"
+    ],
+    "reportDir": "./coverage/node"
+  }
 }

packages/app/src/firebaseApp.ts

@@ -312,14 +312,14 @@ class FirebaseAppImpl implements FirebaseApp {
   /**
    * Return a service instance associated with this app (creating it
    * on demand), identified by the passed instanceIdentifier.
-   * 
+   *
    * NOTE: Currently storage is the only one that is leveraging this
    * functionality. They invoke it by calling:
-   * 
+   *
    * ```javascript
    * firebase.app().storage('STORAGE BUCKET ID')
    * ```
-   * 
+   *
    * The service name is passed to this already
    * @internal
    */
@@ -364,10 +364,10 @@ class FirebaseAppImpl implements FirebaseApp {
     /**
      * If the app has overwritten the addAuthTokenListener stub, forward
      * the active token listeners on to the true fxn.
-     * 
+     *
      * TODO: This function is required due to our current module
      * structure. Once we are able to rely strictly upon a single module
-     * implementation, this code should be refactored and Auth should 
+     * implementation, this code should be refactored and Auth should
      * provide these stubs and the upgrade logic
      */
     if (props.INTERNAL && props.INTERNAL.addAuthTokenListener) {

packages/app/test/firebaseApp.test.ts

@@ -351,8 +351,7 @@ describe('Firebase App Class', () => {
     (app2 as any).test();
     // Confirm extended INTERNAL getToken resolve with the corresponding
     // service's value.
-    return app.INTERNAL
-      .getToken()
+    return app.INTERNAL.getToken()
       .then(token => {
         assert.equal('tokenFor0', token.accessToken);
         return app2.INTERNAL.getToken();

packages/database/index.node.ts

@@ -28,9 +28,9 @@ import './src/nodePatches';
 /**
  * A one off register function which returns a database based on the app and
  * passed database URL.
- * 
+ *
  * @param app A valid FirebaseApp-like object
- * @param url A valid Firebase databaseURL 
+ * @param url A valid Firebase databaseURL
  */
 
 const ServerValue = Database.ServerValue;

packages/database/package.json

@@ -9,7 +9,7 @@
     "dev": "gulp dev",
     "test": "run-p test:browser test:node",
     "test:browser": "karma start --single-run",
-    "test:node": "mocha 'test/{,!(browser)/**/}*.test.ts' --compilers ts:ts-node/register -r src/nodePatches.ts --retries 5 --timeout 5000 --exit",
+    "test:node": "nyc --reporter lcovonly -- mocha 'test/{,!(browser)/**/}*.test.ts' --compilers ts:ts-node/register -r src/nodePatches.ts --retries 5 --timeout 5000 --exit",
     "prepare": "gulp build"
   },
   "license": "Apache-2.0",
@@ -37,6 +37,7 @@
     "karma-webpack": "^2.0.4",
     "mocha": "^4.0.1",
     "npm-run-all": "^4.1.1",
+    "nyc": "^11.2.1",
     "sinon": "^4.0.2",
     "source-map-loader": "^0.2.3",
     "ts-loader": "^3.1.0",
@@ -51,5 +52,11 @@
   "bugs": {
     "url": "https://github.com/firebase/firebase-js-sdk/issues"
   },
-  "typings": "dist/cjs/index.d.ts"
+  "typings": "dist/cjs/index.d.ts",
+  "nyc": {
+    "extension": [
+      ".ts"
+    ],
+    "reportDir": "./coverage/node"
+  }
 }

packages/database/src/core/SyncTree.ts

@@ -546,24 +546,24 @@ export class SyncTree {
   private collectDistinctViewsForSubTree_(
     subtree: ImmutableTree<SyncPoint>
   ): View[] {
-    return subtree.fold<
-      View[]
-    >((relativePath, maybeChildSyncPoint, childMap) => {
-      if (maybeChildSyncPoint && maybeChildSyncPoint.hasCompleteView()) {
-        const completeView = maybeChildSyncPoint.getCompleteView();
-        return [completeView];
-      } else {
-        // No complete view here, flatten any deeper listens into an array
-        let views: View[] = [];
-        if (maybeChildSyncPoint) {
-          views = maybeChildSyncPoint.getQueryViews();
+    return subtree.fold<View[]>(
+      (relativePath, maybeChildSyncPoint, childMap) => {
+        if (maybeChildSyncPoint && maybeChildSyncPoint.hasCompleteView()) {
+          const completeView = maybeChildSyncPoint.getCompleteView();
+          return [completeView];
+        } else {
+          // No complete view here, flatten any deeper listens into an array
+          let views: View[] = [];
+          if (maybeChildSyncPoint) {
+            views = maybeChildSyncPoint.getQueryViews();
+          }
+          forEach(childMap, function(key: string, childViews: View[]) {
+            views = views.concat(childViews);
+          });
+          return views;
         }
-        forEach(childMap, function(key: string, childViews: View[]) {
-          views = views.concat(childViews);
-        });
-        return views;
       }
-    });
+    );
   }
 
   /**

packages/database/src/core/snap/LeafNode.ts

@@ -121,9 +121,10 @@ export class LeafNode implements Node {
     } else if (newChildNode.isEmpty() && childName !== '.priority') {
       return this;
     } else {
-      return LeafNode.__childrenNodeConstructor.EMPTY_NODE
-        .updateImmediateChild(childName, newChildNode)
-        .updatePriority(this.priorityNode_);
+      return LeafNode.__childrenNodeConstructor.EMPTY_NODE.updateImmediateChild(
+        childName,
+        newChildNode
+      ).updatePriority(this.priorityNode_);
     }
   }
 

packages/database/src/core/snap/childSet.ts

@@ -86,7 +86,7 @@ export const buildChildSet = function<K, V>(
       return null;
     } else if (length == 1) {
       namedNode = childList[low];
-      key = keyFn ? keyFn(namedNode) : (namedNode as any) as K;
+      key = keyFn ? keyFn(namedNode) : ((namedNode as any) as K);
       return new LLRBNode(
         key,
         (namedNode.node as any) as V,
@@ -99,7 +99,7 @@ export const buildChildSet = function<K, V>(
       const left = buildBalancedTree(low, middle);
       const right = buildBalancedTree(middle + 1, high);
       namedNode = childList[middle];
-      key = keyFn ? keyFn(namedNode) : (namedNode as any) as K;
+      key = keyFn ? keyFn(namedNode) : ((namedNode as any) as K);
       return new LLRBNode(
         key,
         (namedNode.node as any) as V,
@@ -121,7 +121,7 @@ export const buildChildSet = function<K, V>(
       index -= chunkSize;
       const childTree = buildBalancedTree(low + 1, high);
       const namedNode = childList[low];
-      const key: K = keyFn ? keyFn(namedNode) : (namedNode as any) as K;
+      const key: K = keyFn ? keyFn(namedNode) : ((namedNode as any) as K);
       attachPennant(
         new LLRBNode(key, (namedNode.node as any) as V, color, null, childTree)
       );

packages/database/src/core/util/CountedSet.ts

@@ -29,7 +29,7 @@ export class CountedSet<K, V> {
    * @param {V} val
    */
   add(item: K, val: V) {
-    this.set[item as any] = val !== null ? val : true as any;
+    this.set[item as any] = val !== null ? val : (true as any);
   }
 
   /**

packages/database/src/core/util/SortedMap.ts

@@ -157,9 +157,9 @@ export class LLRBNode<K, V> {
   ) {
     this.color = color != null ? color : LLRBNode.RED;
     this.left =
-      left != null ? left : SortedMap.EMPTY_NODE as LLRBEmptyNode<K, V>;
+      left != null ? left : (SortedMap.EMPTY_NODE as LLRBEmptyNode<K, V>);
     this.right =
-      right != null ? right : SortedMap.EMPTY_NODE as LLRBEmptyNode<K, V>;
+      right != null ? right : (SortedMap.EMPTY_NODE as LLRBEmptyNode<K, V>);
   }
 
   static RED = true;

packages/database/src/core/view/filter/LimitedFilter.ts

@@ -270,7 +270,7 @@ export class LimitedFilter implements NodeFilter {
     const newChildNamedNode = new NamedNode(childKey, childSnap);
     const windowBoundary = this.reverse_
       ? oldEventCache.getFirstChild(this.index_)
-      : oldEventCache.getLastChild(this.index_) as NamedNode;
+      : (oldEventCache.getLastChild(this.index_) as NamedNode);
     const inRange = this.rangedFilter_.matches(newChildNamedNode);
     if (oldEventCache.hasChild(childKey)) {
       const oldChildSnap = oldEventCache.getImmediateChild(childKey);

packages/database/src/realtime/WebSocketConnection.ts

@@ -149,7 +149,9 @@ export class WebSocketConnection implements Transport {
         // UA Format: Firebase/<wire_protocol>/<sdk_version>/<platform>/<device>
         const options: { [k: string]: object } = {
           headers: {
-            'User-Agent': `Firebase/${PROTOCOL_VERSION}/${firebase.SDK_VERSION}/${process.platform}/${device}`
+            'User-Agent': `Firebase/${PROTOCOL_VERSION}/${
+              firebase.SDK_VERSION
+            }/${process.platform}/${device}`
           }
         };
 

packages/database/test/query.test.ts

@@ -83,7 +83,7 @@ describe('Query Tests', function() {
     /**
      * Because we are testing invalid queries, I am casting
      * to `any` to avoid the typechecking error. This can
-     * occur when a user uses the SDK through a pure JS 
+     * occur when a user uses the SDK through a pure JS
      * client, rather than typescript
      */
     expect(function() {