Skip to content

test(filter): enable type inference to marble diagram #2202

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 3 commits into from
Feb 5, 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
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ cache:
env:
matrix:
- NODE_VER=4 FULL_VALIDATE=false
- NODE_VER=6 FULL_VALIDATE=true alias grunt=./node_modules/grunt-cli/bin/grunt
- NODE_VER=6 FULL_VALIDATE=true alias grunt=./node_modules/grunt-cli/bin/grunt danger=./node_modules/danger/distribution/danger
- NODE_VER=7 FULL_VALIDATE=false
matrix:
fast_finish: true
Expand All @@ -23,6 +23,7 @@ before_install:
- nvm install $NODE_VER
- npm install -g npm@4 && node -v && npm -v
- if [ "$FULL_VALIDATE" == "true" ]; then npm install [email protected] grunt-cli grunt-contrib-connect grunt-run; fi
- if [ "$FULL_VALIDATE" == "true" ] && [ -n "DANGER_GITHUB_API_TOKEN" ]; then npm install danger && danger; fi

install:
- npm install
Expand Down
56 changes: 56 additions & 0 deletions dangerfile.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
var fs = require('fs');
var path = require('path');
var _ = require('lodash');

//simple regex matcher to detect usage of helper function and its type signature
var hotMatch = /\bhot\(/gi;
var hotSignatureMatch = /\bdeclare const hot: typeof/gi;

var coldMatch = /\bcold\(/gi;
var coldSignatureMatch = /\bdeclare const cold: typeof/gi;

var errorCount = 0;

// Warn when PR size is large
var bigPRThreshold = 600;
if (danger.github.pr.additions + danger.github.pr.deletions > bigPRThreshold) {
warn(':exclamation: Big PR (' + ++errorCount + ')');
markdown('> (' + errorCount + ') : Pull Request size seems relatively large. If Pull Request contains multiple changes, split each into separate PR will helps faster, easier review.');
}

// Check test exclusion (.only) is included
var modifiedSpecFiles = danger.git.modified_files.filter(function (filePath) {
return filePath.match(/-spec.(js|jsx|ts|tsx)$/gi);
});

var testFilesIncludeExclusion = modifiedSpecFiles.reduce(function (acc, value) {
var content = fs.readFileSync(value).toString();
var invalid = _.includes(content, 'it.only') || _.includes(content, 'describe.only');
if (invalid) {
acc.push(path.basename(value));
}
return acc;
}, []);

if (testFilesIncludeExclusion.length > 0) {
fail('an \`only\` was left in tests (' + testFilesIncludeExclusion + ')');
}

// Check test cases missing type signature import for test marble helper functions
var testFilesMissingTypes = modifiedSpecFiles.reduce(function (acc, value) {
var content = fs.readFileSync(value).toString();

var hotFnMatches = content.match(hotMatch) && content.match(hotSignatureMatch);
var coldFnMatches = content.match(coldMatch) && content.match(coldSignatureMatch);

if (!hotFnMatches || !coldFnMatches) {
acc.push(path.basename(value));
}

return acc;
}, []);

if (testFilesMissingTypes.length > 0) {
fail('missing type definition import in tests (' + testFilesMissingTypes + ') (' + ++errorCount + ')');
markdown('> (' + errorCount + ') : It seems updated test cases uses test scheduler interface `hot`, `cold` but miss to import type signature for those.');
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@
"sinon-chai": "^2.8.0",
"source-map-support": "^0.4.0",
"tslib": "^1.5.0",
"tslint": "^4.2.0",
"tslint": "^4.4.2",
"typescript": "~2.0.6",
"typings": "^2.0.0",
"validate-commit-msg": "^2.3.1",
Expand Down
8 changes: 7 additions & 1 deletion spec/operators/filter-spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import {expect} from 'chai';
import * as Rx from '../../dist/cjs/Rx';
declare const {hot, cold, asDiagram, expectObservable, expectSubscriptions};
import marbleTestingSignature = require('../helpers/marble-testing'); // tslint:disable-line:no-require-imports
Copy link
Member

Choose a reason for hiding this comment

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

How about import * as marbleTestingSignature from '../helpers/marble-testing';?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't tried it but tempted to leave as-is to give clarity this import is unique kind and not being used in general.

Copy link
Member

Choose a reason for hiding this comment

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

That intent may be better expressed as a comment, not as the usage of require.

Copy link
Member

Choose a reason for hiding this comment

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

could you not just destructure it with the import?

import {hot, cold, asDiagram, expectObservable, expectSubscriptions} from '../helpers/marble-testing';

Copy link
Member Author

@kwonoj kwonoj Jan 3, 2017

Choose a reason for hiding this comment

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

No, it's #1633. If use imported module, TS will transpile it and doc will be affected, reason only use it as type to be removed while transpiling.

Copy link
Member

Choose a reason for hiding this comment

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

I think a comment here is super important. I can easily see myself wondering why this was done and changing it later.


declare const { asDiagram };
declare const hot: typeof marbleTestingSignature.hot;
declare const cold: typeof marbleTestingSignature.cold;
declare const expectObservable: typeof marbleTestingSignature.expectObservable;
declare const expectSubscriptions: typeof marbleTestingSignature.expectSubscriptions;

const Observable = Rx.Observable;

Expand Down
2 changes: 1 addition & 1 deletion src/observable/DeferObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class DeferObservable<T> extends Observable<T> {
* }
* });
* clicksOrInterval.subscribe(x => console.log(x));
*
*
* // Results in the following behavior:
* // If the result of Math.random() is greater than 0.5 it will listen
* // for clicks anywhere on the "document"; when document is clicked it
Expand Down
4 changes: 2 additions & 2 deletions src/observable/FromEventPatternObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ export class FromEventPatternObservable<T> extends Observable<T> {
* function addClickHandler(handler) {
* document.addEventListener('click', handler);
* }
*  
*
* function removeClickHandler(handler) {
* document.removeEventListener('click', handler);
* }
*  
*
* var clicks = Rx.Observable.fromEventPattern(
* addClickHandler,
* removeClickHandler
Expand Down
4 changes: 2 additions & 2 deletions src/observable/FromObservable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class FromObservable<T> extends Observable<T> {
* i = 2 * i; // double it
* }
* }
*  
*
* var iterator = generateDoubles(3);
* var result = Rx.Observable.from(iterator).take(10);
* result.subscribe(x => console.log(x));
Expand All @@ -71,7 +71,7 @@ export class FromObservable<T> extends Observable<T> {
* @see {@link fromEvent}
* @see {@link fromEventPattern}
* @see {@link fromPromise}
*  
*
* @param {ObservableInput<T>} ish A subscribable object, a Promise, an
* Observable-like, an Array, an iterable or an array-like object to be
* converted.
Expand Down
2 changes: 1 addition & 1 deletion src/observable/dom/WebSocketSubject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export class WebSocketSubject<T> extends AnonymousSubject<T> {
* @example <caption>Wraps WebSocket from nodejs-websocket (using node.js)</caption>
*
* import { w3cwebsocket } from 'websocket';
*
*
* let socket = new WebSocketSubject({
* url: 'ws://localhost:8081',
* WebSocketCtor: w3cwebsocket
Expand Down
24 changes: 12 additions & 12 deletions src/operator/catch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import { subscribeToResult } from '../util/subscribeToResult';
* Catches errors on the observable to be handled by returning a new observable or throwing an error.
*
* <img src="./img/catch.png" width="100%">
*
*
* @example <caption>Continues with a different Observable when there's an error</caption>
*
*
* Observable.of(1, 2, 3, 4, 5)
* .map(n => {
* .map(n => {
* if (n == 4) {
* throw 'four!';
* }
* return n;
* return n;
* })
* .catch(err => Observable.of('I', 'II', 'III', 'IV', 'V'))
* .subscribe(x => console.log(x));
Expand All @@ -26,25 +26,25 @@ import { subscribeToResult } from '../util/subscribeToResult';
* @example <caption>Retries the caught source Observable again in case of error, similar to retry() operator</caption>
*
* Observable.of(1, 2, 3, 4, 5)
* .map(n => {
* .map(n => {
* if (n === 4) {
* throw 'four!';
* throw 'four!';
* }
* return n;
* return n;
* })
* .catch((err, caught) => caught)
* .take(30)
* .subscribe(x => console.log(x));
* // 1, 2, 3, 1, 2, 3, ...
*
*
* @example <caption>Throws a new error when the source Observable throws an error</caption>
*
*
* Observable.of(1, 2, 3, 4, 5)
* .map(n => {
* .map(n => {
* if (n == 4) {
* throw 'four!';
* }
* return n;
* return n;
* })
* .catch(err => {
* throw 'error in source. Details: ' + err;
Expand All @@ -54,7 +54,7 @@ import { subscribeToResult } from '../util/subscribeToResult';
* err => console.log(err)
* );
* // 1, 2, 3, error in source. Details: four!
*
*
* @param {function} selector a function that takes as arguments `err`, which is the error, and `caught`, which
* is the source observable, in case you'd like to "retry" that observable by returning it again. Whatever observable
* is returned by the `selector` will be used to continue the observable chain.
Expand Down
2 changes: 1 addition & 1 deletion src/operator/concatMapTo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export function concatMapTo<T, I, R>(this: Observable<T>, observable: Observable
* // For every click on the "document" it will emit values 0 to 3 spaced
* // on a 1000ms interval
* // one click = 1000ms-> 0 -1000ms-> 1 -1000ms-> 2 -1000ms-> 3
*
*
* @see {@link concat}
* @see {@link concatAll}
* @see {@link concatMap}
Expand Down
6 changes: 3 additions & 3 deletions src/operator/distinct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,14 @@ import { ISet, Set } from '../util/Set';
* { age: 5, name: 'Foo'})
* .distinct((p: Person) => p.name)
* .subscribe(x => console.log(x));
*
*
* // displays:
* // { age: 4, name: 'Foo' }
* // { age: 7, name: 'Bar' }
*
*
* @see {@link distinctUntilChanged}
* @see {@link distinctUntilKeyChanged}
*
*
* @param {function} [keySelector] optional function to select which value you want to check as distinct.
* @param {Observable} [flushes] optional Observable for flushing the internal HashSet of the operator.
* @return {Observable} an Observable that emits items from the source Observable with distinct values.
Expand Down
4 changes: 2 additions & 2 deletions src/operator/distinctUntilChanged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ export function distinctUntilChanged<T, K>(this: Observable<T>, compare: (x: K,
* { age: 6, name: 'Foo'})
* .distinctUntilChanged((p: Person, q: Person) => p.name === q.name)
* .subscribe(x => console.log(x));
*
*
* // displays:
* // { age: 4, name: 'Foo' }
* // { age: 7, name: 'Bar' }
* // { age: 5, name: 'Foo' }
*
*
* @see {@link distinct}
* @see {@link distinctUntilKeyChanged}
*
Expand Down
6 changes: 3 additions & 3 deletions src/operator/distinctUntilKeyChanged.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@ export function distinctUntilKeyChanged<T, K>(this: Observable<T>, key: string,
/**
* Returns an Observable that emits all items emitted by the source Observable that are distinct by comparison from the previous item,
* using a property accessed by using the key provided to check if the two items are distinct.
*
*
* If a comparator function is provided, then it will be called for each item to test for whether or not that value should be emitted.
*
*
* If a comparator function is not provided, an equality check is used by default.
*
* @example <caption>An example comparing the name of persons</caption>
*
*
* interface Person {
* age: number,
* name: string
Expand Down
2 changes: 1 addition & 1 deletion src/operator/every.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Subscriber } from '../Subscriber';
* Observable.of(1, 2, 3, 4, 5, 6)
* .every(x => x < 5)
* .subscribe(x => console.log(x)); // -> false
*
*
* @param {function} predicate a function for determining if an item meets a specified condition.
* @param {any} [thisArg] optional object to use for `this` in the callback
* @return {Observable} an Observable of booleans that determines if all items of the source Observable meet the condition specified.
Expand Down
2 changes: 1 addition & 1 deletion src/operator/materialize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { Notification } from '../Notification';
* // - Notification {kind: "N", value: "A", error: undefined, hasValue: true}
* // - Notification {kind: "N", value: "B", error: undefined, hasValue: true}
* // - Notification {kind: "E", value: undefined, error: TypeError:
* // x.toUpperCase is not a function at MapSubscriber.letters.map.x
* // x.toUpperCase is not a function at MapSubscriber.letters.map.x
* // [as project] (http://1…, hasValue: false}
*
* @see {@link Notification}
Expand Down
6 changes: 3 additions & 3 deletions src/operator/max.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ReduceOperator } from './reduce';
* Rx.Observable.of(5, 4, 7, 2, 8)
* .max()
* .subscribe(x => console.log(x)); // -> 8
*
*
* @example <caption>Use a comparer function to get the maximal item</caption>
* interface Person {
* age: number,
Expand All @@ -23,9 +23,9 @@ import { ReduceOperator } from './reduce';
* .max<Person>((a: Person, b: Person) => a.age < b.age ? -1 : 1)
* .subscribe((x: Person) => console.log(x.name)); // -> 'Beer'
* }
*
*
* @see {@link min}
*
*
* @param {Function} optional comparer function that it will use instead of its default to compare the value of two
* items.
* @return {Observable} an Observable that emits item with the largest value.
Expand Down
2 changes: 1 addition & 1 deletion src/operator/mergeMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function mergeMap<T, I, R>(this: Observable<T>, project: (value: T, index
* // b1
* // c1
* // continues to list a,b,c with respective ascending integers
*
*
* @see {@link concatMap}
* @see {@link exhaustMap}
* @see {@link merge}
Expand Down
6 changes: 3 additions & 3 deletions src/operator/min.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { ReduceOperator } from './reduce';
* Rx.Observable.of(5, 4, 7, 2, 8)
* .min()
* .subscribe(x => console.log(x)); // -> 2
*
*
* @example <caption>Use a comparer function to get the minimal item</caption>
* interface Person {
* age: number,
Expand All @@ -23,9 +23,9 @@ import { ReduceOperator } from './reduce';
* .min<Person>( (a: Person, b: Person) => a.age < b.age ? -1 : 1)
* .subscribe((x: Person) => console.log(x.name)); // -> 'Bar'
* }
*
*
* @see {@link max}
*
*
* @param {Function} optional comparer function that it will use instead of its default to compare the value of two items.
* @return {Observable<R>} an Observable that emits item with the smallest value.
* @method min
Expand Down
8 changes: 4 additions & 4 deletions src/operator/zip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@ export function zipStatic<R>(...observables: Array<ObservableInput<any> | ((...v
/* tslint:enable:max-line-length */

/**
* Combines multiple Observables to create an Observable whose values are calculated from the values, in order, of each
* Combines multiple Observables to create an Observable whose values are calculated from the values, in order, of each
* of its input Observables.
*
* If the latest parameter is a function, this function is used to compute the created value from the input values.
* If the latest parameter is a function, this function is used to compute the created value from the input values.
* Otherwise, an array of the input values is returned.
*
* @example <caption>Combine age and name from different sources</caption>
Expand All @@ -80,11 +80,11 @@ export function zipStatic<R>(...observables: Array<ObservableInput<any> | ((...v
* (age: number, name: string, isDev: boolean) => ({ age, name, isDev }))
* .subscribe(x => console.log(x));
*
* // outputs
* // outputs
* // { age: 7, name: 'Foo', isDev: true }
* // { age: 5, name: 'Bar', isDev: true }
* // { age: 9, name: 'Beer', isDev: false }
*
*
* @param observables
* @return {Observable<R>}
* @static true
Expand Down