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

[ts][ftr] improve types for ftr and expect.js, cleanup changes to tsconfig files #31948

Merged
merged 24 commits into from
Feb 28, 2019

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Feb 25, 2019

In #31234 there were some extra changes that I've reverted, like use of the tsconfig-paths package to magically rewrite import statements to defy the standard node module resolution algorithm, the inclusion of several unnecessary options in the test/tsconfig.json file, and changes of the line-endings in the config files. This also brings a few enhancements from #30190 including a modularized version of the expect.js types, and options for explicit mappings for the PageObjects and services used in ftr tests.

@spalger spalger added review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc labels Feb 25, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform

@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

Copy link
Member

@azasypkin azasypkin left a comment

Choose a reason for hiding this comment

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

LGTM 👍

packages/kbn-test/tsconfig.json Show resolved Hide resolved
src/es_archiver/es_archiver.d.ts Outdated Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
test/types/ftr_provider_contetxt.d.ts Outdated Show resolved Hide resolved
@elasticmachine

This comment has been minimized.

@spalger
Copy link
Contributor Author

spalger commented Feb 27, 2019

@azasypkin @vitalics I might have gone a little overboard, but the comments @vitalics left inspired me a little bit and I ended up with a variation on the solution I had previously. Now:

  • GenericFtrProviderContext<ServiceProviders, PageObjectProviders> is exported by @kbn/test/types
  • each functional test config/suite defines its own FtrProviderContext, like https://github.com/elastic/kibana/pull/31948/files#diff-f82cd41c7028d7a93e945dac33a144d3R29
  • the services/page_objects directories now export an object of Providers, from an index.ts file, which is the basis for the types used to populate the FtrProviderContext. The type for this object gets merged with the types for the providers from other suites that are used for this services/pageObjects config, and are then passed to GenericFtrProviderContext to create the unique context for each config/suite.

This new structure allows us to have unique FtrProviderContexts for each config/suite so that the api_integration suite can have unique services, like supertest, same as with x-pack/test/functional and so on. I haven't updated the x-pack tests to use these new types yet, I didn't want too much stuff in this PR, but I did sorta stumble around in the functional test runner TypeScript-ifying things, let me know if you'd rather I did that in a separate PR and I can split it out. Sorry for the somewhat massive change to approach this late in the game!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@vitalics vitalics left a comment

Choose a reason for hiding this comment

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

type system did not work as well since we have a es service(EsProvider in common folder), which returns elasticsearch.Client typedef, but when I was trying to use them someone in code, i faced with any issue
image

P.S. common services defined in the right way. I suppose it happens because it "iterable". See ftr.d.ts({[name: string]: FnReturnAny})
Resolved by reverting my local changes 😊

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger spalger merged commit 4749c6a into elastic:master Feb 28, 2019
spalger pushed a commit to spalger/kibana that referenced this pull request Feb 28, 2019
…onfig files (elastic#31948)

In elastic#31234 there were some extra changes that I've reverted, like use of the `tsconfig-paths` package to magically rewrite import statements to defy the standard node module resolution algorithm, the inclusion of several unnecessary options in the `test/tsconfig.json` file, and changes of the line-endings in the config files. This also brings a few enhancements from elastic#30190 including a modularized version of the expect.js types, and options for explicit mappings for the PageObjects and services used in ftr tests.
spalger pushed a commit that referenced this pull request Feb 28, 2019
…onfig files (#31948) (#32250)

In #31234 there were some extra changes that I've reverted, like use of the `tsconfig-paths` package to magically rewrite import statements to defy the standard node module resolution algorithm, the inclusion of several unnecessary options in the `test/tsconfig.json` file, and changes of the line-endings in the config files. This also brings a few enhancements from #30190 including a modularized version of the expect.js types, and options for explicit mappings for the PageObjects and services used in ftr tests.
@spalger
Copy link
Contributor Author

spalger commented Feb 28, 2019

7.x/7.1: ba82d02

@spalger
Copy link
Contributor Author

spalger commented Mar 9, 2019

Backporting to 7.0 in #32827

spalger pushed a commit to spalger/kibana that referenced this pull request Mar 9, 2019
…onfig files (elastic#31948)

In elastic#31234 there were some extra changes that I've reverted, like use of the `tsconfig-paths` package to magically rewrite import statements to defy the standard node module resolution algorithm, the inclusion of several unnecessary options in the `test/tsconfig.json` file, and changes of the line-endings in the config files. This also brings a few enhancements from elastic#30190 including a modularized version of the expect.js types, and options for explicit mappings for the PageObjects and services used in ftr tests.
# Conflicts:
#	src/functional_test_runner/lib/config/schema.js
#	test/common/services/es.ts
#	test/functional/page_objects/index.ts
#	test/functional/services/apps_menu.js
#	yarn.lock
spalger pushed a commit that referenced this pull request Mar 9, 2019
…32827)

* [ts] add script to verify that all ts is in a project (#32727)

Based on #32705 

We currently have TypeScript code that was backported to 7.0, which was backported without issue because it falls outside of any TypeScript projects in 7.0. This means that the pre-commit hooks break on changes to these files, and that they are not getting type checked by the type_check script. To fix this we need to verify that every typescript file in the repository is covered by a tsconfig.json file as part of CI.

* tests typescript migration (#31234)

* add typescript support for functional tests

* [ts][ftr] improve types for ftr and expect.js, cleanup changes to tsconfig files (#31948)

In #31234 there were some extra changes that I've reverted, like use of the `tsconfig-paths` package to magically rewrite import statements to defy the standard node module resolution algorithm, the inclusion of several unnecessary options in the `test/tsconfig.json` file, and changes of the line-endings in the config files. This also brings a few enhancements from #30190 including a modularized version of the expect.js types, and options for explicit mappings for the PageObjects and services used in ftr tests.
# Conflicts:
#	src/functional_test_runner/lib/config/schema.js
#	test/common/services/es.ts
#	test/functional/page_objects/index.ts
#	test/functional/services/apps_menu.js
#	yarn.lock
@spalger spalger added the non-issue Indicates to automation that a pull request should not appear in the release notes label Mar 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
non-issue Indicates to automation that a pull request should not appear in the release notes review Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.0.0 v7.2.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants