[ML] Functional tests - fix typing issue by pheyos · Pull Request #52167 · elastic/kibana

pheyos · 2019-12-04T11:14:17Z

Summary

This PR fixes the typing flakiness in ML wizards, where sometimes the first letter isn't typed in.

This reverts commit e412d7b.

This reverts commit 2fbccc5.

This reverts commit e178fd8.

pheyos · 2019-12-06T12:50:45Z

Testing again with checks during typing

build	results
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/46/	Passed 30x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/47/	Passed 30x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/50/	Passed 40x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/51/	Passed 100x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/52/	Passed 100x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/53/	Passed 100x
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/54/	Passed 100x

elasticmachine · 2019-12-06T14:02:22Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: eb90bff

pheyos · 2019-12-09T08:57:32Z

Summary:

With the first fix attempts, the typing issue reproduced 2-3 times in 60 executions.
The current fix passed 500x in a row

Also, I've discussed this with @jgowdyelastic and we consider this a temporary fix until we find a way to make the job wizard input elements more stable for the automated typing.

elasticmachine · 2019-12-09T08:58:39Z

Pinging @elastic/ml-ui (:ml)

dmlemeshko · 2019-12-09T11:01:25Z

test/functional/services/test_subjects.ts

+              }
+            });
+          });
+        }


I'm not sure if this solution will be effective and stable:

if the problem is that the letter may not be typed, it makes sense to reimplement WebElementWrapper: type function for ML tests. Wrapping existing code with 2 retries will slow things down and an additional check will not give a benefit if the wrong letter or in the letter is typed in the wrong order.

if the problem is that the field may be cleared during typing I suggest checking how the component re-rendering is triggered. Wrapping things with double retry will most like hide an issue the real user can face.

But 500 passing tests means the opposite :)

@dmlemeshko thanks for the feedback!
Some explanations to this approach:

We're already investigating the component and will (hopefully) fix it as soon as we know why it behaves that way, so this PR is only a temporary solution to fix typing until then (this is the 1st reason why I've implemented this as a separate method).

The slowdown due to retries is expected. I've called the method setValueWithChecks to set the right expectations: when I cal something WithChecks, I expect it to be slower (2nd reason for a separate method).

I agree that the outer retry is probably not needed (but I think the same holds true for the setValue method). This will just retry the whole thing in case an error is thrown on the way, so it doesn't really hurt. For the inner retries:

The first block will retry to clear the field

The second block does basically the following:
a. Type in the next character
b. Check if it was appended to the input value (give it 1 second time to appear there)
c. If it doesn't appear, try again, starting with step a. (retry for 5 seconds, then fail)

So I agree that it doesn't help to fix wrong letters or wrong order of letters. But that was not the scope of this fix, it should only check for missing letters.

I can see how this new method setValueWithChecks is very specialized to (hopefully temporarily) fix a specific issue and might not be appropriate for the testSubjects service. Happy to move it to a ML specific location if you think that's better.

@pheyos thank you for explaining the issue, it makes more sense now. Since you mentioned it is a temporary fix for ML-specific components, I suggest we don't put it in testSubjects.ts: we probably don't want other teams to start using it?

@dmlemeshko agreed. I've moved the method to a common ML service in a11a340 .

elasticmachine · 2019-12-09T18:02:17Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: a11a340

History

💚 Build #14125 succeeded a2806d1

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

spalger · 2019-12-09T19:30:28Z

x-pack/test/functional/services/machine_learning/job_wizard_common.ts


 export function MachineLearningJobWizardCommonProvider(
  { getService }: FtrProviderContext,
+  mlCommon: ProvidedType<typeof MachineLearningCommonProvider>,


In the future, I usually like to export this type from the module containing the provider so I can just import the MlCommon type without having to use these long types everywhere.

in common.ts

export type MlCommon = ProvidedType<typeof MachineLearningCommonProvider>

in here

import { MlCommon } from './common'

Good idea! I've created a PR for this: #52612

spalger

LGTM

dmlemeshko

LGTM. Thank you for update

spalger · 2019-12-09T23:00:30Z

I keep running into this, and the changes have two reviews, so I'm going to go ahead and merge this.

* Use char by char typing in all text fields * Add dely before first typed charakter when typing char by char * Remove delay before typing again * Use clearCharByChar option for input fields * Revert "Use clearCharByChar option for input fields" This reverts commit e412d7b. * Revert "Use char by char typing in all text fields" This reverts commit 2fbccc5. * Disable jobCreatorUpdate for tests * Revert "Disable jobCreatorUpdate for tests" This reverts commit e178fd8. * Check typing char by char for job wizard inputs * Remove .only from anomaly detection suite * Move setValueWithChecks from testSubjects to a ML service # Conflicts: # x-pack/test/functional/services/machine_learning/index.ts # x-pack/test/functional/services/machine_learning/job_wizard_common.ts # x-pack/test/functional/services/ml.ts

* Use char by char typing in all text fields * Add dely before first typed charakter when typing char by char * Remove delay before typing again * Use clearCharByChar option for input fields * Revert "Use clearCharByChar option for input fields" This reverts commit e412d7b. * Revert "Use char by char typing in all text fields" This reverts commit 2fbccc5. * Disable jobCreatorUpdate for tests * Revert "Disable jobCreatorUpdate for tests" This reverts commit e178fd8. * Check typing char by char for job wizard inputs * Remove .only from anomaly detection suite * Move setValueWithChecks from testSubjects to a ML service # Conflicts: # x-pack/test/functional/services/machine_learning/index.ts # x-pack/test/functional/services/machine_learning/job_wizard_advanced.ts # x-pack/test/functional/services/machine_learning/job_wizard_common.ts # x-pack/test/functional/services/ml.ts

* Use char by char typing in all text fields * Add dely before first typed charakter when typing char by char * Remove delay before typing again * Use clearCharByChar option for input fields * Revert "Use clearCharByChar option for input fields" This reverts commit e412d7b. * Revert "Use char by char typing in all text fields" This reverts commit 2fbccc5. * Disable jobCreatorUpdate for tests * Revert "Disable jobCreatorUpdate for tests" This reverts commit e178fd8. * Check typing char by char for job wizard inputs * Remove .only from anomaly detection suite * Move setValueWithChecks from testSubjects to a ML service # Conflicts: # x-pack/test/functional/services/machine_learning/index.ts # x-pack/test/functional/services/machine_learning/job_wizard_common.ts # x-pack/test/functional/services/ml.ts

spalger · 2019-12-10T02:10:13Z

7.x/7.6: ea28aa6
7.5: 3b83995

* [ML] Functional tests - fix typing issue (#52167) * Use char by char typing in all text fields * Add dely before first typed charakter when typing char by char * Remove delay before typing again * Use clearCharByChar option for input fields * Revert "Use clearCharByChar option for input fields" This reverts commit e412d7b. * Revert "Use char by char typing in all text fields" This reverts commit 2fbccc5. * Disable jobCreatorUpdate for tests * Revert "Disable jobCreatorUpdate for tests" This reverts commit e178fd8. * Check typing char by char for job wizard inputs * Remove .only from anomaly detection suite * Move setValueWithChecks from testSubjects to a ML service # Conflicts: # x-pack/test/functional/services/machine_learning/index.ts # x-pack/test/functional/services/machine_learning/job_wizard_advanced.ts # x-pack/test/functional/services/machine_learning/job_wizard_common.ts # x-pack/test/functional/services/ml.ts * fix bad merge

pheyos added 2 commits December 4, 2019 11:23

Use char by char typing in all text fields

2fbccc5

Add dely before first typed charakter when typing char by char

d27ad1d

This comment has been minimized.

Sign in to view

pheyos added 3 commits December 5, 2019 11:17

Remove delay before typing again

a9ad526

Use clearCharByChar option for input fields

e412d7b

Merge branch 'master' into fix_typing

d746a35

This comment has been minimized.

Sign in to view

pheyos added 4 commits December 6, 2019 08:46

Revert "Use clearCharByChar option for input fields"

43e9956

This reverts commit e412d7b.

Merge branch 'master' into fix_typing

5db2092

Revert "Use char by char typing in all text fields"

d72f81e

This reverts commit 2fbccc5.

Disable jobCreatorUpdate for tests

e178fd8

This comment has been minimized.

Sign in to view

pheyos added 2 commits December 6, 2019 11:21

Revert "Disable jobCreatorUpdate for tests"

49471bd

This reverts commit e178fd8.

Check typing char by char for job wizard inputs

eb90bff

pheyos added 2 commits December 9, 2019 09:50

Merge branch 'master' into fix_typing

0c759a9

Remove .only from anomaly detection suite

a2806d1

pheyos marked this pull request as ready for review December 9, 2019 08:57

pheyos requested a review from a team as a code owner December 9, 2019 08:57

pheyos requested review from jgowdyelastic and spalger December 9, 2019 08:57

pheyos self-assigned this Dec 9, 2019

pheyos added :ml test_ui_functional labels Dec 9, 2019

pheyos added v7.5.1 v7.6.0 v8.0.0 labels Dec 9, 2019

pheyos added the release_note:skip Skip the PR/issue when compiling release notes label Dec 9, 2019

pheyos requested a review from dmlemeshko December 9, 2019 09:03

dmlemeshko reviewed Dec 9, 2019

View reviewed changes

Move setValueWithChecks from testSubjects to a ML service

a11a340

spalger mentioned this pull request Dec 9, 2019

Add failure screenshot links to JUnit failures #52449

Merged

spalger reviewed Dec 9, 2019

View reviewed changes

spalger approved these changes Dec 9, 2019

View reviewed changes

spalger mentioned this pull request Dec 9, 2019

[ftr/lifecycle] refactor to be typesafe #52453

Merged

dmlemeshko self-requested a review December 9, 2019 20:29

dmlemeshko approved these changes Dec 9, 2019

View reviewed changes

spalger merged commit d429a9a into elastic:master Dec 9, 2019

This was referenced Dec 9, 2019

[7.x] [ML] Functional tests - fix typing issue (#52167) #52581

Merged

[7.5] [ML] Functional tests - fix typing issue (#52167) #52585

Merged

pheyos deleted the fix_typing branch December 10, 2019 10:16

pheyos mentioned this pull request Feb 21, 2020

[ML] Functional tests - stabilize typing during df analytics creation #58227

Merged

Conversation

pheyos commented Dec 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pheyos commented Dec 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Dec 6, 2019

💚 Build Succeeded

Uh oh!

pheyos commented Dec 9, 2019

Uh oh!

elasticmachine commented Dec 9, 2019

Uh oh!

dmlemeshko Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dmlemeshko Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

pheyos Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

dmlemeshko Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

pheyos Dec 9, 2019

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Dec 9, 2019

💚 Build Succeeded

History

Uh oh!

spalger Dec 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pheyos Dec 10, 2019

Choose a reason for hiding this comment

Uh oh!

spalger left a comment

Choose a reason for hiding this comment

Uh oh!

dmlemeshko left a comment

Choose a reason for hiding this comment

Uh oh!

spalger commented Dec 9, 2019

Uh oh!

spalger commented Dec 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pheyos commented Dec 4, 2019 •

edited

Loading

pheyos commented Dec 6, 2019 •

edited

Loading

dmlemeshko Dec 9, 2019 •

edited

Loading

spalger Dec 9, 2019 •

edited

Loading

spalger commented Dec 10, 2019 •

edited

Loading