Skip to content

Instrument Repo for Code Coverage #268

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Nov 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,7 @@ dist
.idea
.vscode
*.iml

# Coverage
.nyc_output
coverage
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion config/karma.base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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']
}
};

Expand Down
15 changes: 12 additions & 3 deletions config/webpack.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const path = require('path');
const webpack = require('webpack');

module.exports = {
devtool: 'eval-source-map',
devtool: 'source-map',
module: {
rules: [
{
Expand All @@ -27,9 +27,18 @@ module.exports = {
use: 'ts-loader'
},
{
test: /\.js$/,
use: ['source-map-loader'],
test: /\.[tj]sx?$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just a standard configuration now or are we really doing jsx and tsx stuff? In contrast in other locations we're still using just straight .ts file extensions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just a standard thing, we have no need for jsx/tsx so it isn't that big of a deal I don't think.

use: 'source-map-loader',
enforce: 'pre'
},
{
test: /\.tsx?$/,
use: {
loader: 'istanbul-instrumenter-loader',
options: { esModules: true }
},
enforce: 'post',
include: path.resolve(process.cwd(), 'src')
}
]
},
Expand Down
4 changes: 2 additions & 2 deletions integration/firestore/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down
8 changes: 4 additions & 4 deletions integration/shared/validator.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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>
* }
* }
* }
* }
*/
Expand Down
6 changes: 6 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -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",
Expand Down
13 changes: 11 additions & 2 deletions packages/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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"
}
}
10 changes: 5 additions & 5 deletions packages/app/src/firebaseApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 1 addition & 2 deletions packages/app/test/firebaseApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions packages/database/index.node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 9 additions & 2 deletions packages/database/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand All @@ -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"
}
}
32 changes: 16 additions & 16 deletions packages/database/src/core/SyncTree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions packages/database/src/core/snap/LeafNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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_);
}
}

Expand Down
6 changes: 3 additions & 3 deletions packages/database/src/core/snap/childSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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)
);
Expand Down
2 changes: 1 addition & 1 deletion packages/database/src/core/util/CountedSet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/database/src/core/util/SortedMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/database/src/core/view/filter/LimitedFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion packages/database/src/realtime/WebSocketConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`
}
};

Expand Down
2 changes: 1 addition & 1 deletion packages/database/test/query.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
Loading