[Uptime] Update Ml functional test #62562

shahzad31 · 2020-04-06T07:48:30Z

Summary

Fixes: #60963

Added functional test for ML

Testing

This doesn't change any functionality, basically if CI is good, this should be good to go.

elasticmachine · 2020-04-06T07:49:01Z

Pinging @elastic/uptime (Team:uptime)

shahzad31 · 2020-04-08T11:51:43Z

@elasticmachine merge upstream

justinkambic

Had a few comments and asks for clarification. Thanks for taking the time to write this!

justinkambic · 2020-04-09T19:35:39Z

x-pack/test/functional/services/uptime/ml_anomaly.ts

+  };
+
+  return {
+    async openMLFlyoutOrMenu() {


I'm struggling to understand what's happening here. Why would we want to test only one of the two options, wouldn't this mean the outcome of the test could be different on subsequent runs but still pass?

justinkambic · 2020-04-09T20:43:10Z

x-pack/test/functional/apps/uptime/ml_anomaly.ts

+      }
+    });
+
+    it('open ml flyout or menu', async () => {


I commented on the function in the uptime service, but based on the code it seems like this test could have more than one expected outcome, which seems strange to me.

justinkambic · 2020-04-09T20:47:11Z

x-pack/test/functional/apps/uptime/ml_anomaly.ts

+    });
+
+    it('can create and delete ML anomaly job', async () => {
+      if (await uptime.ml.alreadyHasJob()) {


I think we could simplify this, by just having

if (await uptime.ml.alreadyHasJob()) { await uptime.ml.deleteMLJob(); } await uptime.ml.createMLJob(); await uptime.navigation.refreshApp(); await uptime.ml.deleteMLJob();

Then the test will always have the same outcome.

justinkambic · 2020-04-09T20:53:07Z

x-pack/test/functional/services/uptime/common.ts

+        this.setStatusFilterDown();
+      }
+
+      return true;


Is it necessary to return true if we're never going to return a different result? Promise<void> seems more appropriate.

justinkambic · 2020-04-09T20:54:36Z

x-pack/test/functional/services/uptime/ml_anomaly.ts

+  const retry = getService('retry');
+  const log = getService('log');
+
+  const alreadyHasJob = async () => {


Is there a reason the contents of this function can't live in the exported function below that is calling it?

justinkambic · 2020-04-09T20:58:16Z

x-pack/test/functional/services/uptime/ml_anomaly.ts

+      });
+    },
+    async canCreateJob() {
+      return !!(await (await testSubjects.find('uptimeMLCreateJobBtn')).getAttribute('disabled'));


Suggested change

return !!(await (await testSubjects.find('uptimeMLCreateJobBtn')).getAttribute('disabled'));

const createJobBtn = await testSubjects.find('uptimeMLCreateJobBtn');

return !!await createJobBtn.getAttribute('disabled');

I think breaking this up to two lines will make it easier to understand what is happening.

justinkambic · 2020-04-09T20:59:08Z

x-pack/test/functional/services/uptime/navigation.ts

-      await testSubjects.existOrFail('uptimeOverviewPage', { timeout: 2000 });
+    // Check if are already on overview uptime page, we don't need to repeat the step
+    await retry.tryForTime(60 * 1000, async () => {
+      if (await testSubjects.exists('uptimeSettingsToOverviewLink', { timeout: 0 })) {


Suggested change

if (await testSubjects.exists('uptimeSettingsToOverviewLink', { timeout: 0 })) {

if (await testSubjects.exists('uptimeSettingsToOverviewLink')) {

That parameter is optional, if we aren't specifying a timeout anymore it's not needed.

if we don't specify timeout 0, it uses default timeout from config.

justinkambic · 2020-04-09T20:59:54Z

x-pack/test/functional/services/uptime/navigation.ts

+      if (await testSubjects.exists('uptimeSettingsToOverviewLink', { timeout: 0 })) {
+        await testSubjects.click('uptimeSettingsToOverviewLink');
+        await testSubjects.existOrFail('uptimeOverviewPage', { timeout: 2000 });
+      } else if (!(await testSubjects.exists('uptimeOverviewPage', { timeout: 0 }))) {


Suggested change

} else if (!(await testSubjects.exists('uptimeOverviewPage', { timeout: 0 }))) {

} else if (!(await testSubjects.exists('uptimeOverviewPage'))) {

Same comment as above.

justinkambic · 2020-04-09T21:19:45Z

x-pack/test/functional/services/uptime/navigation.ts

-      ) {
-        throw new Error('Expected monitor name not found');
+    checkIfOnMonitorPage: async (monitorId?: string) => {
+      const monitorPage = await testSubjects.exists('uptimeMonitorPage', { timeout: 0 });


Suggested change

const monitorPage = await testSubjects.exists('uptimeMonitorPage', { timeout: 0 });

const monitorPage = await testSubjects.exists('uptimeMonitorPage');

justinkambic · 2020-04-09T21:20:22Z

x-pack/test/functional/services/uptime/navigation.ts

-        (await testSubjects.getVisibleText('monitor-page-title')) !== monitorName
-      ) {
-        throw new Error('Expected monitor name not found');
+    checkIfOnMonitorPage: async (monitorId?: string) => {


Suggested change

checkIfOnMonitorPage: async (monitorId?: string) => {

checkIfOnMonitorPage: async (monitorId: string) => {

Any reason we can't require this parameter?

…to ml-func-test

shahzad31 · 2020-04-10T16:38:18Z

@justinkambic took care of your feedback.

justinkambic

LGTM

shahzad31 · 2020-04-13T10:02:54Z

@elasticmachine merge upstream

kibanamachine · 2020-04-13T11:48:38Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 85054da

History

💚 Build #40182 succeeded fe51033
💛 Build #39561 was flaky 2f47cfb
💚 Build #38963 succeeded 041c199
💔 Build #38786 failed 86f4ac5
💔 Build #38771 failed 50fcbe0

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

* update test * added test * updated type * updated test * updated test * update test Co-authored-by: Elastic Machine <[email protected]>

* update test * added test * updated type * updated test * updated test * update test Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>

shahzad31 added 3 commits April 4, 2020 15:39

update test

2551fd3

Merge branch 'master' into ml-func-test

848b332

added test

0997848

shahzad31 self-assigned this Apr 6, 2020

shahzad31 added release_note:skip Skip the PR/issue when compiling release notes Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.8.0 v8.0.0 labels Apr 6, 2020

shahzad31 added 3 commits April 6, 2020 10:54

Merge branch 'master' into ml-func-test

bad171f

updated type

50fcbe0

updated test

86f4ac5

shahzad31 marked this pull request as ready for review April 6, 2020 10:20

shahzad31 requested a review from justinkambic April 6, 2020 10:20

updated test

041c199

shahzad31 requested a review from andrewvc April 6, 2020 18:39

Merge branch 'master' into ml-func-test

2f47cfb

justinkambic reviewed Apr 9, 2020

View reviewed changes

shahzad31 added 3 commits April 10, 2020 18:31

update test

046f6a0

Merge branch 'ml-func-test' of https://github.com/shahzad31/kibana in…

647fbc5

…to ml-func-test

Merge branch 'master' into ml-func-test

fe51033

justinkambic approved these changes Apr 10, 2020

View reviewed changes

Merge branch 'master' into ml-func-test

85054da

shahzad31 merged commit c7f61f9 into elastic:master Apr 13, 2020

shahzad31 deleted the ml-func-test branch April 13, 2020 11:49

shahzad31 added a commit to shahzad31/kibana that referenced this pull request Apr 13, 2020

[Uptime] Update Ml functional test (elastic#62562)

978131b

* update test * added test * updated type * updated test * updated test * update test Co-authored-by: Elastic Machine <[email protected]>

shahzad31 mentioned this pull request Apr 13, 2020

[7.x] [Uptime] Update Ml functional test (#62562) #63336

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Uptime] Update Ml functional test #62562

[Uptime] Update Ml functional test #62562

shahzad31 commented Apr 6, 2020

elasticmachine commented Apr 6, 2020

shahzad31 commented Apr 8, 2020

justinkambic left a comment

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

shahzad31 Apr 10, 2020

justinkambic Apr 10, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

justinkambic Apr 9, 2020

shahzad31 commented Apr 10, 2020

justinkambic left a comment

shahzad31 commented Apr 13, 2020

kibanamachine commented Apr 13, 2020

	return !!(await (await testSubjects.find('uptimeMLCreateJobBtn')).getAttribute('disabled'));
	const createJobBtn = await testSubjects.find('uptimeMLCreateJobBtn');
	return !!await createJobBtn.getAttribute('disabled');

	if (await testSubjects.exists('uptimeSettingsToOverviewLink', { timeout: 0 })) {
	if (await testSubjects.exists('uptimeSettingsToOverviewLink')) {

	} else if (!(await testSubjects.exists('uptimeOverviewPage', { timeout: 0 }))) {
	} else if (!(await testSubjects.exists('uptimeOverviewPage'))) {

	const monitorPage = await testSubjects.exists('uptimeMonitorPage', { timeout: 0 });
	const monitorPage = await testSubjects.exists('uptimeMonitorPage');

	checkIfOnMonitorPage: async (monitorId?: string) => {
	checkIfOnMonitorPage: async (monitorId: string) => {

[Uptime] Update Ml functional test #62562

[Uptime] Update Ml functional test #62562

Conversation

shahzad31 commented Apr 6, 2020

Summary

Testing

elasticmachine commented Apr 6, 2020

shahzad31 commented Apr 8, 2020

justinkambic left a 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

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

shahzad31 commented Apr 10, 2020

justinkambic left a comment

Choose a reason for hiding this comment

shahzad31 commented Apr 13, 2020

kibanamachine commented Apr 13, 2020

💚 Build Succeeded

History