Skip to content
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

Increase dtslint test coverage #4093

Closed
cartant opened this issue Sep 5, 2018 · 7 comments
Closed

Increase dtslint test coverage #4093

cartant opened this issue Sep 5, 2018 · 7 comments
Labels
help wanted Issues we wouldn't mind assistance with.

Comments

@cartant
Copy link
Collaborator

cartant commented Sep 5, 2018

Chore

#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

@cartant cartant added the help wanted Issues we wouldn't mind assistance with. label Sep 5, 2018
This was referenced Sep 15, 2018
dkosasih added a commit to dkosasih/rxjs that referenced this issue Jan 29, 2019
dkosasih added a commit to dkosasih/rxjs that referenced this issue Jan 29, 2019
dkosasih added a commit to dkosasih/rxjs that referenced this issue Jan 29, 2019
dkosasih added a commit to dkosasih/rxjs that referenced this issue Jan 29, 2019
benlesh pushed a commit that referenced this issue Jan 30, 2019
* test(dtslint): add dtslint test for forkjoin obsrevable (#4093)

* chore(forkJoin): update dtslint with TODOs for what needs fixed
dkosasih added a commit to dkosasih/rxjs that referenced this issue Feb 28, 2019
benlesh pushed a commit that referenced this issue Aug 1, 2019
* test(dtslint): add dtslint test for merge operator (#4093)

* test(dtslint): change dtslint test for merge operator to use helpers (#4093)
benlesh pushed a commit that referenced this issue Aug 1, 2019
* test(dtslint): add dtslint test for merge operator (#4093)

* test(dtslint): change dtslint test for merge operator to use helpers (#4093)
benlesh pushed a commit that referenced this issue Aug 1, 2019
)

* test(dtslint): add dtslint test for of observable static function (#4093)

* test(dtslint): change of observable static function dtslint test to use helper (#4093)
benlesh pushed a commit that referenced this issue Aug 1, 2019
)

* test(dtslint): add dtslint test for of observable static function (#4093)

* test(dtslint): change of observable static function dtslint test to use helper (#4093)
dkosasih added a commit to dkosasih/rxjs that referenced this issue Aug 2, 2019
dkosasih pushed a commit to dkosasih/rxjs that referenced this issue Aug 2, 2019
benlesh pushed a commit that referenced this issue Dec 11, 2019
* chore(typings): fix iif observable typings for optional parameters (#4494)

* test(dtslint): add dtslint test for iif observable static function (#4093)

* chore: remove unnecessary signature

* chore: fix dtslint expectations

Closes #4494

* chore: empty commit to kick CI
benlesh pushed a commit that referenced this issue Dec 11, 2019
* chore(typings): fix iif observable typings for optional parameters (#4494)

* test(dtslint): add dtslint test for iif observable static function (#4093)

* chore: remove unnecessary signature

* chore: fix dtslint expectations

Closes #4494

* chore: empty commit to kick CI
@kwonoj
Copy link
Member

kwonoj commented Jan 22, 2021

Guess this is good to close? @cartant

@cartant
Copy link
Collaborator Author

cartant commented Jan 22, 2021

Yep.

@cartant cartant closed this as completed Jan 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues we wouldn't mind assistance with.
Projects
None yet
Development

No branches or pull requests

4 participants