Skip to content

Commit

Permalink
[ML] Functional tests - fix and re-enable validation API tests
Browse files Browse the repository at this point in the history
  • Loading branch information
pheyos committed Oct 15, 2020
1 parent 0c7ca14 commit 5b68f12
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 15 deletions.
32 changes: 26 additions & 6 deletions x-pack/test/api_integration/apis/ml/job_validation/cardinality.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand Down Expand Up @@ -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 () => {
Expand Down
40 changes: 31 additions & 9 deletions x-pack/test/api_integration/apis/ml/job_validation/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -236,7 +234,7 @@ export default ({ getService }: FtrProviderContext) => {
}
});

expect(body).to.eql([
const expectedResponse = [
{
id: 'job_id_valid',
heading: 'Job ID format is valid',
Expand All @@ -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',
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit 5b68f12

Please sign in to comment.