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

Convert test suite to TS #478

Merged
merged 5 commits into from Feb 3, 2020
Merged

Convert test suite to TS #478

merged 5 commits into from Feb 3, 2020

Conversation

ro0gr
Copy link
Collaborator

@ro0gr ro0gr commented Jan 21, 2020

This would allow to truly test our public APIs, w/o relying on synthetic dtslint tests.
An intention is to only convert tests which don't use deprecated(or almost deprecated) APIs. Even this way quite few typings issues were detected and some of them fixed in this PR.

Unfortunately I had to pin to ec-ts@1, cause on a later versions ec-babel@7 or higher is required, and it requires node@6 or higher. And that would be a breaking change, cause currently we have node@4.5 listed in our engines. So upgrade of ec-ts should be done with the next major version.

todo:

  • finish migration and make it green
  • make sure if we don't publish any of new TS stuff

@ro0gr ro0gr force-pushed the ts-tests branch 2 times, most recently from a0beff1 to 33d5913 Compare January 22, 2020 23:02
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noEmitOnError": true,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This fails the build in case of any TS build error occurs for both development and CI.
It may be not convenient for everyone, especially for people who are new with TS, or those who just want to quickly experiment and not to fight with types.

So, if anyone knows a way to only enable such behavior on Travis, hints/suggestions are welcome!

@coveralls
Copy link

coveralls commented Jan 22, 2020

Coverage Status

Coverage remained the same at 98.65% when pulling ccf9119 on ro0gr:ts-tests into 873e1cd on san650:master.

@ro0gr ro0gr marked this pull request as ready for review January 23, 2020 00:03
It uses ec-typescript@1, cause more recent versions of ec-typescipt
require ember-cli-babel@7 or higher. And that requires node@6
or higher, which is technically a breaking change, cause we
support node@4.5 in v1

However, we use blueprint setup from ec-ts@3 here to have the best
we can for now.

The other changes:
  - Setup build to fail on TS error
  - remove dtslint
  - Add loader.js "require" types
  - Some of test helpers which are time consuming to truly convert got
    ambient types, since it's not really critical for testing our public
    APIs
  - Simpler test helpers were converted to TS
@ro0gr ro0gr force-pushed the ts-tests branch 2 times, most recently from d068b4c to 7c4b3d2 Compare January 24, 2020 22:42
Caveats:

 - "multiple" on props is allowed to be un-typed, cause this behavior ha
 to be deprecated
 - Allow legacy collection API to be un-typed
    - Add "key" arg to "getter("
    - Add "chainable" option to the "alias("
    - Add missing "scope" option to FindOneOptions
    - Fix "triggerable(" options
    - Fix "text(" options
    - Add "selactable(" types
    - Assume "objectAt(" always returns a found page object instance
@ro0gr ro0gr merged commit f999eba into san650:master Feb 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants