Skip to content

Commit

Permalink
[ML] Functional tests - fix and re-enable validation API tests (elast…
Browse files Browse the repository at this point in the history
…ic#80617)

This PR fixes and re-enables the recently disabled job validation API tests that validate cardinalities.
  • Loading branch information
pheyos committed Oct 15, 2020
1 parent 0fb76aa commit f802084
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 15 deletions.
34 changes: 28 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 @@ -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');
Expand Down Expand Up @@ -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: '',
Expand Down Expand Up @@ -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 () => {
Expand Down
42 changes: 33 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 @@ -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');
Expand Down Expand Up @@ -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: {
Expand Down Expand Up @@ -236,7 +236,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 +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',
Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit f802084

Please sign in to comment.