Skip to content

WIP: refactor(database): Add migrated test harness [Part 3/3] #71

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 35 commits into from
Jul 6, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
4a0df56
refactor(database): add typescript implementation
jshcrowthe Jun 16, 2017
dcf2f5c
refactor(database): update build process to include database.ts
jshcrowthe Jun 16, 2017
7e609d6
refactor(*): refactor environment builds to be based on separate .ts …
jshcrowthe Jun 16, 2017
e72bfbe
WIP: patch database code in nodeJS
jshcrowthe Jun 16, 2017
2f6a406
refactor(database): classes for typescript database implementation (#55)
jsayol Jun 17, 2017
19519ac
WIP: add the /tests/config dir to the .gitignore
jshcrowthe Jun 19, 2017
2758ca7
WIP: add test harness
jshcrowthe Jun 19, 2017
7f78f89
WIP: add query tests
jshcrowthe Jun 19, 2017
b12a61f
WIP: add database.test.ts
jshcrowthe Jun 19, 2017
a046261
WIP: add node.test.ts
jshcrowthe Jun 19, 2017
622fed8
WIP: add sortedmap.test.ts
jshcrowthe Jun 19, 2017
d35b183
WIP: add datasnapshot.test.ts
jshcrowthe Jun 19, 2017
c81583d
WIP: add sparsesnapshottree.test.ts
jshcrowthe Jun 19, 2017
f4fc618
refactor(database): refactor query.test.ts to better preserve origina…
jshcrowthe Jun 20, 2017
342d527
WIP: add crawler_support.test.ts
jshcrowthe Jun 20, 2017
c51999a
WIP: refactor EventAccumulator.ts for data.test.ts
jshcrowthe Jun 21, 2017
c0eba9c
WIP: fix issue with query.test.ts
jshcrowthe Jun 22, 2017
e290f67
WIP: add connection.test.ts
jshcrowthe Jun 22, 2017
c0af997
WIP: add info.test.ts
jshcrowthe Jun 22, 2017
1223678
WIP: add order_by.test.ts
jshcrowthe Jun 22, 2017
9e08172
WIP: fix several code signature problems, add test files
jshcrowthe Jun 22, 2017
379a263
WIP: add transaction.test.ts
jshcrowthe Jun 26, 2017
3193651
WIP: working on the broken npm test command
jshcrowthe Jun 27, 2017
d5f99d8
WIP: working on fixes
jshcrowthe Jun 27, 2017
58f8bff
WIP: remove logging
jshcrowthe Jun 27, 2017
869b9a5
WIP: fix node tests
jshcrowthe Jun 27, 2017
ebe491c
fix(*): fixing test files and CI integration
jshcrowthe Jun 28, 2017
2b1efa4
WIP: tEMP: Allow branch builds
jshcrowthe Jun 28, 2017
c40a189
WIP: escape string
jshcrowthe Jun 28, 2017
d78d6c4
refactor(CI): use ChromeHeadless launcher
jshcrowthe Jun 28, 2017
2939025
Merge branch 'ts-database/final' into ts-database/migrate-test-harness
jshcrowthe Jun 28, 2017
d214ba0
WIP: fixes from review.
jshcrowthe Jun 29, 2017
dc5c374
WIP: skip flakey test
jshcrowthe Jun 29, 2017
71ff51c
WIP: remove unneeded debugger statement
jshcrowthe Jun 29, 2017
2531901
WIP: fixing nits
jshcrowthe Jul 5, 2017
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
5 changes: 3 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
npm-debug.log
/coverage
/.nyc_output
/tests/integration/config
/tests/config
/temp
/.vscode
/.vscode
/.ts-node
4 changes: 1 addition & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ addons:
- g++-4.8
before_script:
- "export DISPLAY=:99.0"
- "mkdir -p tests/config && echo \"$PROJECT_CONFIG\" > tests/config/project.json"
script:
- xvfb-run npm test
branches:
only:
- master
21 changes: 18 additions & 3 deletions gulp/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,13 @@ const path = require('path');
const cwd = process.cwd();
const karma = require('karma');

module.exports = {
const configObj = {
root: path.resolve(cwd),
pkg: require(path.resolve(cwd, 'package.json')),
testConfig: {
timeout: 5000,
retries: 5
},
tsconfig: require(path.resolve(cwd, 'tsconfig.json')),
tsconfigTest: require(path.resolve(cwd, 'tsconfig.test.json')),
paths: {
Expand All @@ -30,6 +34,7 @@ module.exports = {
'tests/**/*.test.ts',
'!tests/**/browser/**/*.test.ts',
'!tests/**/binary/**/*.test.ts',
'!src/firebase-*.ts',
],
binary: [
'tests/**/binary/**/*.test.ts',
Expand Down Expand Up @@ -102,7 +107,7 @@ module.exports = {

// start these browsers
// available browser launchers: https://npmjs.org/browse/keyword/karma-launcher
browsers: ['Chrome', 'Firefox'],
browsers: ['ChromeHeadless', 'Firefox'],

// Continuous Integration mode
// if true, Karma captures browsers, runs the tests and exits
Expand All @@ -115,6 +120,16 @@ module.exports = {
// karma-typescript config
karmaTypescriptConfig: {
tsconfig: `./tsconfig.test.json`
},

// Stub for client config
client: {
mocha: {}
}
}
};
};

configObj.karma.client.mocha.timeout = configObj.testConfig.timeout;
configObj.karma.client.mocha.retries = configObj.testConfig.retries;

module.exports = configObj;
9 changes: 4 additions & 5 deletions gulp/tasks/dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,18 @@ const gulp = require('gulp');
const config = require('../config');

// Ensure that the test tasks get set up
require('./test');
const testFxns = require('./test');

function watchDevFiles() {
const stream = gulp.watch([
`${config.root}/src/**/*.ts`,
config.paths.test.unit
], gulp.parallel('test:unit'));
'tests/**/*.test.ts'
], testFxns.runBrowserUnitTests(true));

stream.on('error', () => {});
stream.on('error', err => {});
return stream;
}

gulp.task('dev', gulp.parallel([
'test:unit',
watchDevFiles
]));
62 changes: 39 additions & 23 deletions gulp/tasks/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ function runNodeUnitTests() {
.pipe(envs)
.pipe(mocha({
reporter: 'spec',
compilers: 'ts:ts-node/register'
compilers: 'ts:ts-node/register',
timeout: config.testConfig.timeout,
retries: config.testConfig.retries
}));
}

Expand All @@ -47,33 +49,41 @@ function runNodeBinaryTests() {
.pipe(envs)
.pipe(mocha({
reporter: 'spec',
compilers: 'ts:ts-node/register'
compilers: 'ts:ts-node/register',
timeout: config.testConfig.timeout,
retries: config.testConfig.retries
}));
}

/**
* Runs all of the browser unit tests in karma
*/
function runBrowserUnitTests(done) {
const karmaConfig = Object.assign({}, config.karma, {
// list of files / patterns to load in the browser
files: [
'./+(src|tests)/**/*.ts'
],

// list of files to exclude from the included globs above
exclude: [
// we don't want this file as it references files that only exist once compiled
`./src/firebase.ts`,
function runBrowserUnitTests(dev) {
return (done) => {
const karmaConfig = Object.assign({}, config.karma, {
// list of files / patterns to load in the browser
files: [
'./+(src|tests)/**/*.ts'
],

// list of files to exclude from the included globs above
exclude: [
// we don't want this file as it references files that only exist once compiled
`./src/firebase-*.ts`,

// Don't include node test files
'./tests/**/node/**/*.test.ts',
// We don't want to load the node env
`./src/utils/nodePatches.ts`,

// Don't include binary test files
'./tests/**/binary/**/*.test.ts',
],
});
new karma.Server(karmaConfig, done).start();
// Don't include node test files
'./tests/**/node/**/*.test.ts',

// Don't include binary test files
'./tests/**/binary/**/*.test.ts',
],
browsers: !!dev ? ['ChromeHeadless'] : config.karma.browsers,
});
new karma.Server(karmaConfig, done).start();
};
}

/**
Expand Down Expand Up @@ -111,7 +121,10 @@ function runAllKarmaTests(done) {
// list of files to exclude from the included globs above
exclude: [
// we don't want this file as it references files that only exist once compiled
`./src/firebase.ts`,
`./src/firebase-*.ts`,

// We don't want to load the node env
`./src/utils/nodePatches.ts`,

// Don't include node test files
'./tests/**/node/**/*.test.ts',
Expand All @@ -121,9 +134,9 @@ function runAllKarmaTests(done) {
}

gulp.task('test:unit:node', runNodeUnitTests);
gulp.task('test:unit:browser', runBrowserUnitTests);
gulp.task('test:unit:browser', runBrowserUnitTests());

const unitTestSuite = gulp.parallel(runNodeUnitTests, runBrowserUnitTests);
const unitTestSuite = gulp.parallel(runNodeUnitTests, runBrowserUnitTests());
gulp.task('test:unit', unitTestSuite);

gulp.task('test:binary:browser', runBrowserBinaryTests);
Expand All @@ -137,3 +150,6 @@ gulp.task('test', gulp.parallel([
runNodeBinaryTests,
runAllKarmaTests
]));

exports.runNodeUnitTests = runNodeUnitTests;
exports.runBrowserUnitTests = runBrowserUnitTests;
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
"babel-preset-env": "^1.2.1",
"chai": "^3.5.0",
"child-process-promise": "^2.2.1",
"cross-env": "^5.0.1",
"cz-customizable": "^5.0.0",
"filesize": "^3.5.6",
"git-rev-sync": "^1.9.0",
Expand Down
10 changes: 9 additions & 1 deletion src/database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,15 @@ declare module './app/firebase_app' {

declare module './app/firebase_app' {
interface FirebaseNamespace {
database?(app: FirebaseApp): Database
database?: {
(app?: FirebaseApp): Database,
Database,
enableLogging,
INTERNAL,
Query,
Reference,
ServerValue,
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/database/api/Database.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export class Database {
* @param {string=} pathString
* @return {!Firebase} Firebase reference.
*/
ref(pathString?): Reference {
ref(pathString?: string): Reference {
this.checkDeleted_('ref');
validateArgCount('database.ref', 0, 1, arguments.length);

Expand Down
5 changes: 3 additions & 2 deletions src/database/api/Query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,8 +221,9 @@ export class Query {
* @param context
* @return {!firebase.Promise}
*/
once(eventType: string, userCallback: SnapshotCallback,
cancelOrContext?, context?: Object) {
once(eventType: string,
userCallback?: SnapshotCallback,
cancelOrContext?, context?: Object): Promise<DataSnapshot> {
validateArgCount('Query.once', 1, 4, arguments.length);
validateEventType('Query.once', 1, eventType, false);
validateCallback('Query.once', 2, userCallback, true);
Expand Down
10 changes: 5 additions & 5 deletions src/database/api/onDisconnect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class OnDisconnect {
}

/**
* @param {function(?Error)=} opt_onComplete
* @param {function(?Error)=} onComplete
* @return {!firebase.Promise}
*/
cancel(onComplete?) {
Expand All @@ -38,7 +38,7 @@ export class OnDisconnect {
}

/**
* @param {function(?Error)=} opt_onComplete
* @param {function(?Error)=} onComplete
* @return {!firebase.Promise}
*/
remove(onComplete?) {
Expand All @@ -52,7 +52,7 @@ export class OnDisconnect {

/**
* @param {*} value
* @param {function(?Error)=} opt_onComplete
* @param {function(?Error)=} onComplete
* @return {!firebase.Promise}
*/
set(value, onComplete?) {
Expand All @@ -68,7 +68,7 @@ export class OnDisconnect {
/**
* @param {*} value
* @param {number|string|null} priority
* @param {function(?Error)=} opt_onComplete
* @param {function(?Error)=} onComplete
* @return {!firebase.Promise}
*/
setWithPriority(value, priority, onComplete?) {
Expand All @@ -86,7 +86,7 @@ export class OnDisconnect {

/**
* @param {!Object} objectToMerge
* @param {function(?Error)=} opt_onComplete
* @param {function(?Error)=} onComplete
* @return {!firebase.Promise}
*/
update(objectToMerge, onComplete?) {
Expand Down
5 changes: 3 additions & 2 deletions src/database/core/Repo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,8 +286,9 @@ export class Repo {
* @param {number|string|null} newPriority
* @param {?function(?Error, *=)} onComplete
*/
setWithPriority(path: Path, newVal: any, newPriority: number | string | null,
onComplete: ((status: Error | null, errorReason?: string) => any) | null) {
setWithPriority(path: Path, newVal: any,
newPriority: number | string | null,
onComplete: ((status: Error | null, errorReason?: string) => any) | null) {
this.log_('set', {path: path.toString(), value: newVal, priority: newPriority});

// TODO: Optimize this behavior to either (a) store flag to skip resolving where possible and / or
Expand Down
16 changes: 8 additions & 8 deletions src/database/core/util/SortedMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -714,35 +714,35 @@ export class SortedMap {
* @param {(function(K, V):T)=} opt_resultGenerator
* @return {SortedMapIterator.<K, V, T>} The iterator.
*/
getIterator(opt_resultGenerator) {
getIterator(resultGenerator?) {
return new SortedMapIterator(this.root_,
null,
this.comparator_,
false,
opt_resultGenerator);
resultGenerator);
}

getIteratorFrom(key, opt_resultGenerator) {
getIteratorFrom(key, resultGenerator?) {
return new SortedMapIterator(this.root_,
key,
this.comparator_,
false,
opt_resultGenerator);
resultGenerator);
}

getReverseIteratorFrom(key, opt_resultGenerator) {
getReverseIteratorFrom(key, resultGenerator?) {
return new SortedMapIterator(this.root_,
key,
this.comparator_,
true,
opt_resultGenerator);
resultGenerator);
}

getReverseIterator(opt_resultGenerator) {
getReverseIterator(resultGenerator?) {
return new SortedMapIterator(this.root_,
null,
this.comparator_,
true,
opt_resultGenerator);
resultGenerator);
}
}; // end SortedMap
2 changes: 1 addition & 1 deletion src/database/core/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export const buildLogMessage_ = function(var_args) {
* Use this for all debug messages in Firebase.
* @type {?function(string)}
*/
export var logger = console.log.bind(console);
export var logger = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the value is originally null. I set it because I was using the logging everywhere.



/**
Expand Down
2 changes: 1 addition & 1 deletion src/database/core/view/filter/RangedFilter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IndexedFilter } from "./IndexedFilter";
import { PRIORITY_INDEX } from "../../../core/snap/indexes/PriorityIndex";
import { NamedNode } from "../../../core/snap/Node";
import { Node, NamedNode } from "../../../core/snap/Node";
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem to be needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Typescript throws an "Unknown Symbol" compiler error if it isn't present :(

import { ChildrenNode } from "../../../core/snap/ChildrenNode";
/**
* Filters nodes by range and uses an IndexFilter to track any changes after filtering the node
Expand Down
Loading