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

chore(test): start fixing test types #3680

Merged
merged 5 commits into from
May 21, 2018
Merged

Conversation

cartant
Copy link
Collaborator

@cartant cartant commented May 10, 2018

Description:

At the moment, the test script runs the specs using mocha and uses ts-node/register to transpile the TypeScript. However, the configuration does not enable type checking and there are many, many errors.

This PR adds some temporary changes to the infrastructure to facilitate the incremental fixing of these type errors. Specifically it:

  • Creates a temporary spec/tsconfig.check.json file.
  • Specifies an include in said file so that files can be incrementally checked and fixed.
  • Creates a temporary script named test_check in the package.json to run a no-emit tsc build that uses spec/tsconfig.check.json.
  • Includes npm run test_check in the .travis.yml configuration file so that the CI build runs it.

With these changes it's possible to incrementally fix specific spec files - by adding them to the includes property in the spec/tsconfig.check.json file. This PR includes fixes for several files. However, fixes are still outstanding for all of the spec files in:

  • spec/observables
  • spec/operators
  • spec/schedulers
  • spec/subjects

The intention is for PRs to include individual commits for specific files. For example, add ./operators/buffer-spec.ts to the include property, fix any type errors, then include those fixes in a commit.

When all -spec.ts files pass, the temporary scripts can be removed from the package.json and the .travis.yml files and the spec/tsconfig.check.json file can be deleted.

At that stage, the TS_NODE_TYPE_CHECK environment variable can be added to the test script so that ts-node will throw an error if a TypeScript error is introduced into the tests.

Related issue (if exists): #3411

@coveralls
Copy link

Coverage Status

Coverage remained the same at 96.833% when pulling da7bf69 on cartant:issue-3411 into dc66731 on ReactiveX:master.

@benlesh benlesh merged commit 7f9fadd into ReactiveX:master May 21, 2018
@benlesh
Copy link
Member

benlesh commented May 21, 2018

Thanks, @cartant! This is a solid add.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants