From 5b68f129e4677cb5cfac692ce24753f7a201f4de Mon Sep 17 00:00:00 2001 From: Robert Oskamp Date: Thu, 15 Oct 2020 09:21:56 +0200 Subject: [PATCH] [ML] Functional tests - fix and re-enable validation API tests --- .../apis/ml/job_validation/cardinality.ts | 32 ++++++++++++--- .../apis/ml/job_validation/validate.ts | 40 ++++++++++++++----- 2 files changed, 57 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..bbec55ece0436 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 @@ -60,9 +60,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 +94,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: 'WILL BE VALIDATED SEPARATELY BELOW', + }, { 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..511ccdaa6681d 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 @@ -178,9 +178,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 +234,7 @@ export default ({ getService }: FtrProviderContext) => { } }); - expect(body).to.eql([ + const expectedResponse = [ { id: 'job_id_valid', heading: 'Job ID format is valid', @@ -254,10 +252,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: 'WILL BE VALIDATED SEPARATELY BELOW', + text: 'WILL BE VALIDATED SEPARATELY BELOW', + status: 'WILL BE VALIDATED SEPARATELY BELOW', }, { id: 'cardinality_partition_field', @@ -298,7 +295,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 () => {