[ML] Advanced wizard functional UI tests #49480

pheyos · 2019-10-28T14:04:23Z

Summary

This PR adds tests for the ML advanced wizard.

High level test steps

Create a new advanced job, validate wizards controls on the way
Run the job and wait for job processing to finish
Validate job details in the job list
Clone that job, validate pre-filled wizard fields
Run the clone job and wait for job processing to finish
Validate clone job details in the job list

These steps are executed for two jobs:

one with a couple metric detectors to cover some functions and the different the usage of field / by field / over field / partition field
- high_count
- mean("products.base_price") by "category.keyword
- sum("products.discount_amount") over customer_id
- median(total_quantity) partition_field_name=customer_gender
- max(total_quantity) by "geoip.continent_name" over customer_id partition_field_name=customer_gender
one with a categorization field and a corresponding detector count by mlcategory

Other changes

Add a couple data-test-subj attributes to the ML UI
Add detector result validation all existing anomaly detection job tests
Adjust tests to handle the changed empty combo boxes correctly
Add needed service methods
A couple stability fixes

elasticmachine · 2019-10-28T14:04:26Z

Pinging @elastic/ml-ui (:ml)

jgowdyelastic · 2019-10-28T14:12:31Z

...obs/new_job_new/pages/components/pick_fields_step/components/advanced_view/detector_list.tsx

@@ -166,3 +172,7 @@ const DuplicateDetectorsWarning: FC<{ validation: Validation }> = ({ validation
    </Fragment>
  );
 };
+
+const DetectorIdentifier: FC<{ detector: Detector }> = ({ detector }) => {


i'm not sure about the name DetectorIdentifier.
It's not obvious what it'll produce.
The component produces a string describing the detector. something like StringifiedDetector is closer to what it's doing, but that's not great either.

Renamed in f32ca31

jgowdyelastic · 2019-10-28T14:22:52Z

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

@@ -8,10 +8,24 @@ import expect from '@kbn/expect';
 import { isEmpty } from 'lodash';
 import { FtrProviderContext } from '../../ftr_provider_context';

+export enum JobState {


There are const enums in ML which could be used instead of defining your own.
https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/ml/common/constants/states.ts

If it's not possible to import from ML like that, i'd suggest you copy the style and make these constants upper case.

Great hint, thanks! Changed it in f32ca31

jgowdyelastic · 2019-10-28T14:22:58Z

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

@@ -8,10 +8,24 @@ import expect from '@kbn/expect';
 import { isEmpty } from 'lodash';
 import { FtrProviderContext } from '../../ftr_provider_context';

+export enum JobState {


There are const enums in ML which could be used instead of defining your own.
https://github.com/elastic/kibana/blob/master/x-pack/legacy/plugins/ml/common/constants/states.ts

If it's not possible to import from ML like that, i'd suggest you copy the style and make these constants upper case.

jgowdyelastic · 2019-10-28T14:24:13Z

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

+        .then((res: any) => res.body);
+
+      expect(jobStats.jobs).to.have.length(1);
+      const state = jobStats.jobs[0].state as JobState;


type assertion isn't needed here. this could be:
const state: JobState = jobStats.jobs[0].state;
or const state: JOB_STATE = jobStats.jobs[0].state; if the previous suggested change is made.

Done in f32ca31

jgowdyelastic · 2019-10-28T14:24:59Z

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

+        .then((res: any) => res.body);
+
+      expect(datafeedStats.datafeeds).to.have.length(1);
+      const state = datafeedStats.datafeeds[0].state as DatafeedState;


same suggestion as before about removing the as

Done in f32ca31

elasticmachine · 2019-10-28T15:34:29Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 242be04

alvarezmelissa87

Overall, LGTM - approving to avoid being a blocker once the enum and typescript changes have been done.

jgowdyelastic

LGTM

walterra

LGTM, just added suggestions/questions about some subject namings.

walterra · 2019-10-29T08:30:08Z

...obs/new_job_new/pages/components/pick_fields_step/components/advanced_view/detector_list.tsx

            <EuiPanel paddingSize="m">
              <EuiFlexGroup>
                <EuiFlexItem>
                  {d.detector_description !== undefined ? (
-                    <div style={{ fontWeight: 'bold' }}>{d.detector_description}</div>
+                    <div style={{ fontWeight: 'bold' }} data-test-subj="detectorDescription">


Should this subject be mlDetectorDescription?

It was originally planned to be nested subject only as it's not necessarily unique on its own, i.e. it's currently used as

mlAdvancedDetector ${detectorIndex} > detectorIdentifier

but it makes sense to keep our ml namespace here as well.

Renamed in dfe2e17

walterra · 2019-10-29T08:30:51Z

...obs/new_job_new/pages/components/pick_fields_step/components/advanced_view/detector_list.tsx

@@ -166,3 +172,7 @@ const DuplicateDetectorsWarning: FC<{ validation: Validation }> = ({ validation
    </Fragment>
  );
 };
+
+const StringifiedDetector: FC<{ detector: Detector }> = ({ detector }) => {
+  return <div data-test-subj="detectorIdentifier">{detectorToString(detector)}</div>;


Should this subject be mlDetectorIdentifier?

See above, renamed in dfe2e17

elasticmachine · 2019-10-29T09:21:30Z

💔 Build Failed

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

elasticmachine · 2019-10-29T11:20:48Z

💚 Build Succeeded

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

This PR adds tests for the ML advanced wizard.

pheyos added 12 commits October 28, 2019 07:55

Add data-test-subj attributes

09a65c9

Add more service functions and update existing ones

e46b3ff

Add jobWizardAdvanced service

292f2f7

Add test to create advanced job

25174f1

Fix advanced section selector

543e7cc

Add detector results existence check

5ef0edb

Add advanced job cloning and support for detector descriptions

c58dca7

Add validations for datafeed config fields

594cc45

Support optional datafeed config values / default values

178a449

Refactor combo box assertions to take string[] as parameter

38cb287

Removed not needed fields from ml/farequote archive mapping

dca9a83

Use ecommerce data instead of farequote, add categorization job

cf6fc45

pheyos added :ml test_ui_functional v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Oct 28, 2019

pheyos requested review from walterra, alvarezmelissa87 and jgowdyelastic October 28, 2019 14:04

pheyos requested a review from a team as a code owner October 28, 2019 14:04

pheyos self-assigned this Oct 28, 2019

jgowdyelastic reviewed Oct 28, 2019

View reviewed changes

Remove debug comments and suite restriction

242be04

jgowdyelastic reviewed Oct 28, 2019

View reviewed changes

alvarezmelissa87 approved these changes Oct 28, 2019

View reviewed changes

Use ML states enums, rename detector FC

f32ca31

jgowdyelastic approved these changes Oct 29, 2019

View reviewed changes

walterra approved these changes Oct 29, 2019

View reviewed changes

Add ml namespace to subj names

dfe2e17

pheyos merged commit 75384e1 into elastic:master Oct 29, 2019

pheyos deleted the advanced_wizard_tests branch October 29, 2019 12:36

pheyos mentioned this pull request Oct 29, 2019

[7.x] [ML] Advanced wizard functional UI tests (#49480) #49623

Merged

pheyos added a commit to pheyos/kibana that referenced this pull request Oct 29, 2019

[ML] Advanced wizard functional UI tests (elastic#49480)

98bba32

This PR adds tests for the ML advanced wizard.

pheyos mentioned this pull request Oct 29, 2019

[7.5] [ML] Advanced wizard functional UI tests (#49480) #49624

Merged

pheyos added a commit to pheyos/kibana that referenced this pull request Oct 29, 2019

[ML] Advanced wizard functional UI tests (elastic#49480)

dddaf2b

This PR adds tests for the ML advanced wizard.

pheyos added a commit that referenced this pull request Oct 29, 2019

[ML] Advanced wizard functional UI tests (#49480) (#49623)

b1f497f

This PR adds tests for the ML advanced wizard.

pheyos added a commit that referenced this pull request Oct 29, 2019

[ML] Advanced wizard functional UI tests (#49480) (#49624)

1a4a7b0

This PR adds tests for the ML advanced wizard.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Advanced wizard functional UI tests #49480

[ML] Advanced wizard functional UI tests #49480

pheyos commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

jgowdyelastic Oct 28, 2019

pheyos Oct 29, 2019

jgowdyelastic Oct 28, 2019

pheyos Oct 29, 2019

jgowdyelastic Oct 28, 2019

jgowdyelastic Oct 28, 2019

pheyos Oct 29, 2019

jgowdyelastic Oct 28, 2019

pheyos Oct 29, 2019

elasticmachine commented Oct 28, 2019

alvarezmelissa87 left a comment •

edited

Loading

jgowdyelastic left a comment

walterra left a comment

walterra Oct 29, 2019

pheyos Oct 29, 2019 •

edited

Loading

pheyos Oct 29, 2019

walterra Oct 29, 2019

pheyos Oct 29, 2019

elasticmachine commented Oct 29, 2019

elasticmachine commented Oct 29, 2019

[ML] Advanced wizard functional UI tests #49480

[ML] Advanced wizard functional UI tests #49480

Conversation

pheyos commented Oct 28, 2019

Summary

High level test steps

Other changes

elasticmachine commented Oct 28, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 28, 2019

💚 Build Succeeded

alvarezmelissa87 left a comment • edited Loading

Choose a reason for hiding this comment

jgowdyelastic left a comment

Choose a reason for hiding this comment

walterra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pheyos Oct 29, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 29, 2019

💔 Build Failed

elasticmachine commented Oct 29, 2019

💚 Build Succeeded

alvarezmelissa87 left a comment •

edited

Loading

pheyos Oct 29, 2019 •

edited

Loading