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

Functional tests: convert more test/services to TS #45176

Merged
merged 7 commits into from
Sep 11, 2019

Conversation

dmlemeshko
Copy link
Member

@dmlemeshko dmlemeshko commented Sep 9, 2019

Summary

This PR migrates services located in test/services to typescript.
I added small descriptions to some functions to clarify how they should be used.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko
Copy link
Member Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@dmlemeshko dmlemeshko added release_note:skip Skip the PR/issue when compiling release notes v7.3.2 v7.4.1 v7.5.0 v8.0.0 labels Sep 10, 2019
@dmlemeshko dmlemeshko marked this pull request as ready for review September 10, 2019 14:19
Co-Authored-By: Tre' <wayne.seymour@elastic.co>
Copy link
Member

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

Great work @dmlemeshko , lemme know if we need to zoom or discuss the type signatures.

test/functional/services/combo_box.ts Outdated Show resolved Hide resolved
test/functional/services/combo_box.ts Outdated Show resolved Hide resolved
test/functional/services/embedding.ts Show resolved Hide resolved
const testSubjects = getService('testSubjects');
import { FtrProviderContext } from '../ftr_provider_context';

export function FilterBarProvider({ getService, getPageObjects }: FtrProviderContext) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function FilterBarProvider({ getService, getPageObjects }: FtrProviderContext) {
export function FilterBarProvider({ getService, getPageObjects }: FtrProviderContext): InstanceType<FilterBar> {

Again, not sure if this is how to properly type a factory function in TS.

export function FlyoutProvider({ getService }) {
import { FtrProviderContext } from '../ftr_provider_context';

export function FlyoutProvider({ getService }: FtrProviderContext) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function FlyoutProvider({ getService }: FtrProviderContext) {
export function FlyoutProvider({ getService }: FtrProviderContext): InstanceType<typeof Flyout> {

Maybe?

const RENDER_COMPLETE_SELECTOR = '[data-render-complete="true"]';
const RENDER_COMPLETE_PENDING_SELECTOR = '[data-render-complete="false"]';
const DATA_LOADING_SELECTOR = '[data-loading]';

export function RenderableProvider({ getService }) {
export function RenderableProvider({ getService }: FtrProviderContext) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function RenderableProvider({ getService }: FtrProviderContext) {
export function RenderableProvider({ getService }: FtrProviderContext): InstanceType<typeof Renderable> {

throw new Error(`${
completedElements.length
} elements completed rendering, still waiting on a total of ${count}
specifically:\n${pendingElementNames.join('\n')}`);
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious, did prettier or eslint cause this formatting?
Not interested in bikeshedding on formatting, but it seemed peculiar.

test/functional/services/table.ts Show resolved Hide resolved
export function VisualizeListingTableProvider({ getService, getPageObjects }) {
import { FtrProviderContext } from '../ftr_provider_context';

export function VisualizeListingTableProvider({ getService, getPageObjects }: FtrProviderContext) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
export function VisualizeListingTableProvider({ getService, getPageObjects }: FtrProviderContext) {
export function VisualizeListingTableProvider({ getService, getPageObjects }: FtrProviderContext): InstanceType<typeof VisualizeListingTable> {

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

IMO, specifying explicit return values when they can be inferred is unnecessary. But changes LGTM

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@wayneseymour
Copy link
Member

LGTM

Copy link
Member

@wayneseymour wayneseymour left a comment

Choose a reason for hiding this comment

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

LGTM

@dmlemeshko dmlemeshko merged commit 8a900bf into elastic:master Sep 11, 2019
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
dmlemeshko added a commit to dmlemeshko/kibana that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 11, 2019
…-to-np-ready

* 'master' of github.com:elastic/kibana:
  Upgrade EUI to 13.8.1 (elastic#45052)
  [ML] Add multi metric job wizard test (elastic#45279)
  [SIEM] Inject/apply KQL changed in refresh button (elastic#45065)
  [Graph] Type persistence (elastic#44985)
  Functional tests: convert more test/services to TS (elastic#45176)
dmlemeshko added a commit that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 11, 2019
…ditor

* 'master' of github.com:elastic/kibana: (76 commits)
  Upgrade EUI to 13.8.1 (elastic#45052)
  [ML] Add multi metric job wizard test (elastic#45279)
  [SIEM] Inject/apply KQL changed in refresh button (elastic#45065)
  [Graph] Type persistence (elastic#44985)
  Functional tests: convert more test/services to TS (elastic#45176)
  [ML] Fixes display of matching modules in index data visualizer (elastic#45261)
  [Console] Update indentation behaviour (elastic#45249)
  Convert value provided to PhraseValueInput to string to catch Exception (elastic#45259)
  [Region Map] Fix loading default vector map and base layer setting (elastic#43858)
  [ML] Fixing empty time range when cloning jobs (elastic#45286)
  [ML] Fixing wizard validation delay (elastic#45265)
  [Logs UI] Interpret finished analysis jobs as healthy (elastic#45268)
  [Console] SQL template with triple quote in completion (elastic#45248)
  [ML] Data Frames: Cards as links (elastic#45254)
  fix(code/frontend): should show updating instead of cloning when updating (elastic#45238)
  fix(code/frontend): fix document search result from (elastic#45236)
  disable another flaky suite (elastic#45323) (elastic#45330)
  disable flaky suite (elastic#45105)
  skip flaky suite (elastic#43069)
  skip flaky suite (elastic#45089)
  ...
dmlemeshko added a commit that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
dmlemeshko added a commit that referenced this pull request Sep 11, 2019
* convert more test/services to TS

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* Update test/functional/services/combo_box.ts

Co-Authored-By: Tre' <wayne.seymour@elastic.co>

* fix lint error
@dmlemeshko
Copy link
Member Author

7.x 5c3cd1b
7.4 cb681ed
7.3 d4c28ec

@dmlemeshko dmlemeshko deleted the tsfy-test-services branch January 31, 2022 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.3.2 v7.4.1 v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants