From f802084b020e7a9f8f6f25c351e88aac587992f5 Mon Sep 17 00:00:00 2001 From: Robert Oskamp Date: Thu, 15 Oct 2020 14:40:19 +0200 Subject: [PATCH] [ML] Functional tests - fix and re-enable validation API tests (#80617) This PR fixes and re-enables the recently disabled job validation API tests that validate cardinalities. --- .../apis/ml/job_validation/cardinality.ts | 34 ++++++++++++--- .../apis/ml/job_validation/validate.ts | 42 +++++++++++++++---- 2 files changed, 61 insertions(+), 15 deletions(-) diff --git a/x-pack/test/api_integration/apis/ml/job_validation/cardinality.ts b/x-pack/test/api_integration/apis/ml/job_validation/cardinality.ts index 00c1ae12e182a..f7657e482d87d 100644 --- a/x-pack/test/api_integration/apis/ml/job_validation/cardinality.ts +++ b/x-pack/test/api_integration/apis/ml/job_validation/cardinality.ts @@ -13,6 +13,8 @@ export default ({ getService }: FtrProviderContext) => { const supertest = getService('supertestWithoutAuth'); const ml = getService('ml'); + const VALIDATED_SEPARATELY = 'this value is not validated directly'; + describe('ValidateCardinality', function () { before(async () => { await esArchiver.loadIfNeeded('ml/ecommerce'); @@ -60,9 +62,7 @@ export default ({ getService }: FtrProviderContext) => { expect(body).to.eql([{ id: 'success_cardinality' }]); }); - // Failing ES promotion due to changes in the cardinality agg, - // see https://github.com/elastic/kibana/issues/80418 - it.skip(`should recognize a high model plot cardinality`, async () => { + it(`should recognize a high model plot cardinality`, async () => { const requestBody = { job_id: '', description: '', @@ -96,10 +96,32 @@ export default ({ getService }: FtrProviderContext) => { .send(requestBody) .expect(200); - expect(body).to.eql([ - { id: 'cardinality_model_plot_high', modelPlotCardinality: 4711 }, + const expectedResponse = [ + { + id: 'cardinality_model_plot_high', + modelPlotCardinality: VALIDATED_SEPARATELY, + }, { id: 'cardinality_partition_field', fieldName: 'order_id' }, - ]); + ]; + + expect(body.length).to.eql( + expectedResponse.length, + `Response body should have ${expectedResponse.length} entries (got ${body})` + ); + for (const entry of expectedResponse) { + const responseEntry = body.find((obj: any) => obj.id === entry.id); + expect(responseEntry).to.not.eql( + undefined, + `Response entry with id '${entry.id}' should exist` + ); + + if (entry.id === 'cardinality_model_plot_high') { + // don't check the exact value of modelPlotCardinality as this is an approximation + expect(responseEntry).to.have.property('modelPlotCardinality'); + } else { + expect(responseEntry).to.eql(entry); + } + } }); it('should not validate cardinality in case request payload is invalid', async () => { diff --git a/x-pack/test/api_integration/apis/ml/job_validation/validate.ts b/x-pack/test/api_integration/apis/ml/job_validation/validate.ts index 01e3da64a515d..8f78cdf015601 100644 --- a/x-pack/test/api_integration/apis/ml/job_validation/validate.ts +++ b/x-pack/test/api_integration/apis/ml/job_validation/validate.ts @@ -14,6 +14,8 @@ export default ({ getService }: FtrProviderContext) => { const supertest = getService('supertestWithoutAuth'); const ml = getService('ml'); + const VALIDATED_SEPARATELY = 'this value is not validated directly'; + describe('Validate job', function () { before(async () => { await esArchiver.loadIfNeeded('ml/ecommerce'); @@ -178,9 +180,7 @@ export default ({ getService }: FtrProviderContext) => { ]); }); - // Failing ES promotion due to changes in the cardinality agg, - // see https://github.com/elastic/kibana/issues/80418 - it.skip('should recognize non-basic issues in job configuration', async () => { + it('should recognize non-basic issues in job configuration', async () => { const requestBody = { duration: { start: 1586995459000, end: 1589672736000 }, job: { @@ -236,7 +236,7 @@ export default ({ getService }: FtrProviderContext) => { } }); - expect(body).to.eql([ + const expectedResponse = [ { id: 'job_id_valid', heading: 'Job ID format is valid', @@ -254,10 +254,9 @@ export default ({ getService }: FtrProviderContext) => { }, { id: 'cardinality_model_plot_high', - modelPlotCardinality: 4711, - text: - 'The estimated cardinality of 4711 of fields relevant to creating model plots might result in resource intensive jobs.', - status: 'warning', + modelPlotCardinality: VALIDATED_SEPARATELY, + text: VALIDATED_SEPARATELY, + status: VALIDATED_SEPARATELY, }, { id: 'cardinality_partition_field', @@ -298,7 +297,32 @@ export default ({ getService }: FtrProviderContext) => { url: `https://www.elastic.co/guide/en/machine-learning/${pkg.branch}/create-jobs.html#model-memory-limits`, status: 'warning', }, - ]); + ]; + + expect(body.length).to.eql( + expectedResponse.length, + `Response body should have ${expectedResponse.length} entries (got ${body})` + ); + for (const entry of expectedResponse) { + const responseEntry = body.find((obj: any) => obj.id === entry.id); + expect(responseEntry).to.not.eql( + undefined, + `Response entry with id '${entry.id}' should exist` + ); + + if (entry.id === 'cardinality_model_plot_high') { + // don't check the exact value of modelPlotCardinality as this is an approximation + expect(responseEntry).to.have.property('modelPlotCardinality'); + expect(responseEntry) + .to.have.property('text') + .match( + /^The estimated cardinality of [0-9]+ of fields relevant to creating model plots might result in resource intensive jobs./ + ); + expect(responseEntry).to.have.property('status', 'warning'); + } else { + expect(responseEntry).to.eql(entry); + } + } }); it('should not validate configuration in case request payload is invalid', async () => {