ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.63k stars 3k forks source link

Increase dtslint test coverage #4093

Closed cartant closed 3 years ago

cartant commented 6 years ago

Chore

https://github.com/ReactiveX/rxjs/pull/3944 added dtslint to the Travis $FULL_VALIDATE configuration. However, the coverage of the codebase is far from complete.

PRs that contribute to increasing the coverage are welcome.

The purpose of the dtslint tests is to ensure that the types are inferred as expected and to ensure that type errors are effected for invalid types. At the moment, there are some type tests in the codebase. These are able to do the former, but not the latter - as type errors would break the build.

For an example of the type tests, see these for the zip observable and these for the zip operator. The type tests are not executed when the tests run. Rather, they're tested when the tests are built. And if the tests build, the type tests have passed. Not all operators and operators have type tests. In fact, many don't.

The dtslint tests reside in the spec-dtslint directory. They are written as Mocha-style it tests, but they are not run. Instead, TSLint runs rules that test the inferred types against expectations that are expression in comments. For example:

it('should support observables', () => {
  const a = of(1); // $ExpectType Observable<number>
  const b = of('foo'); // $ExpectType Observable<string>
  const c = of(true); // $ExpectType Observable<boolean>
  const o1 = zip(a, b, c); // $ExpectType Observable<[number, string, boolean]>
});

You can find the type tests that have been converted to dtslint tests for the zip observable here and for the zip operator here.

Apart from converting the type tests, we want to ensure that there are dtslint tests for each overload signature for each observable and operator.

Submitting separate PRs - each with one or only a few observables or operators - would be the best way to approach this, as PRs with a limited number of observables or operators should be straightforward to review.

Running the tests

The tests will run on the Travis CI build, but you can run them locally using the dtslint script:

npm run dtslint

However, doing so will see dtslint run via npx, which means it will be downloaded each time the script runs. This is very slow, as it also downloads several versions of TypeScript.

If you are going to run dtslint locally, you might want to install it globally and then run the tests using:

dtslint spec-dtslint/

Additional context

timdeschryver commented 5 years ago

We're past half-way through! ✋ @dkosasih

timdeschryver commented 5 years ago

And of course you too @cartant, thanks for the reviews.

timdeschryver commented 5 years ago

These are the operators that need coverage. As you can see, we're almost there 😄

timdeschryver commented 5 years ago

A copy, since the previous message was hidden.

These are the operators that need coverage. As you can see, we're almost there 😄

dkosasih commented 5 years ago

Merry Christmas everyone. I just want to list down the observable creator function from the source so we can all refer to the list and not cross path while working on it.

kwonoj commented 3 years ago

Guess this is good to close? @cartant

cartant commented 3 years ago

Yep.