From e39ef039dd51a5d691a0743bf33f757e160962d8 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Mon, 18 May 2020 14:58:36 +0200 Subject: [PATCH 1/2] Allow histogram fields in average and sum aggs --- .../data/public/search/aggs/metrics/avg.ts | 2 +- .../data/public/search/aggs/metrics/sum.ts | 2 +- .../page_objects/visualize_chart_page.ts | 1 + .../apps/visualize/precalculated_histogram.ts | 75 +++++++++++++------ 4 files changed, 54 insertions(+), 26 deletions(-) diff --git a/src/plugins/data/public/search/aggs/metrics/avg.ts b/src/plugins/data/public/search/aggs/metrics/avg.ts index 96be3e849a3e8..dba18d562ec19 100644 --- a/src/plugins/data/public/search/aggs/metrics/avg.ts +++ b/src/plugins/data/public/search/aggs/metrics/avg.ts @@ -51,7 +51,7 @@ export const getAvgMetricAgg = ({ getInternalStartServices }: AvgMetricAggDepend { name: 'field', type: 'field', - filterFieldTypes: KBN_FIELD_TYPES.NUMBER, + filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM], }, ], }, diff --git a/src/plugins/data/public/search/aggs/metrics/sum.ts b/src/plugins/data/public/search/aggs/metrics/sum.ts index 70fc379f2d5f1..66fad89316613 100644 --- a/src/plugins/data/public/search/aggs/metrics/sum.ts +++ b/src/plugins/data/public/search/aggs/metrics/sum.ts @@ -54,7 +54,7 @@ export const getSumMetricAgg = ({ getInternalStartServices }: SumMetricAggDepend { name: 'field', type: 'field', - filterFieldTypes: KBN_FIELD_TYPES.NUMBER, + filterFieldTypes: [KBN_FIELD_TYPES.NUMBER, KBN_FIELD_TYPES.HISTOGRAM], }, ], }, diff --git a/test/functional/page_objects/visualize_chart_page.ts b/test/functional/page_objects/visualize_chart_page.ts index 71e722a9c8fdd..d562f20265563 100644 --- a/test/functional/page_objects/visualize_chart_page.ts +++ b/test/functional/page_objects/visualize_chart_page.ts @@ -312,6 +312,7 @@ export function VisualizeChartPageProvider({ getService, getPageObjects }: FtrPr /** * If you are writing new tests, you should rather look into getTableVisContent method instead. + * @deprecated Use getTableVisContent instead. */ public async getTableVisData() { return await testSubjects.getVisibleText('paginated-table-body'); diff --git a/x-pack/test/functional/apps/visualize/precalculated_histogram.ts b/x-pack/test/functional/apps/visualize/precalculated_histogram.ts index 5d362d29b640c..b012ca34b3ab1 100644 --- a/x-pack/test/functional/apps/visualize/precalculated_histogram.ts +++ b/x-pack/test/functional/apps/visualize/precalculated_histogram.ts @@ -24,37 +24,64 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { return esArchiver.unload('pre_calculated_histogram'); }); - const initHistogramBarChart = async () => { - await PageObjects.visualize.navigateToNewVisualization(); - await PageObjects.visualize.clickVerticalBarChart(); - await PageObjects.visualize.clickNewSearch('histogram-test'); - await PageObjects.visChart.waitForVisualization(); - }; - - const getFieldOptionsForAggregation = async (aggregation: string): Promise => { - await PageObjects.visEditor.clickBucket('Y-axis', 'metrics'); - await PageObjects.visEditor.selectAggregation(aggregation, 'metrics'); - const fieldValues = await PageObjects.visEditor.getField(); - return fieldValues; - }; - it('appears correctly in discover', async function() { await PageObjects.common.navigateToApp('discover'); const rowData = await PageObjects.discover.getDocTableIndex(1); expect(rowData.includes('"values": [ 0.3, 1, 3, 4.2, 4.8 ]')).to.be.ok(); }); - it('appears in the field options of a Percentiles aggregation', async function() { - await initHistogramBarChart(); - const fieldValues: string[] = await getFieldOptionsForAggregation('Percentiles'); - log.debug('Percentiles Fields = ' + fieldValues); - expect(fieldValues[0]).to.be('histogram-content'); - }); + describe('works in visualizations', () => { + before(async () => { + await PageObjects.visualize.navigateToNewVisualization(); + await PageObjects.visualize.clickDataTable(); + await PageObjects.visualize.clickNewSearch('histogram-test'); + await PageObjects.visChart.waitForVisualization(); + await PageObjects.visEditor.clickMetricEditor(); + }); + + const renderTableForAggregation = async ( + aggregation: string + ): Promise => { + await PageObjects.visEditor.selectAggregation(aggregation, 'metrics'); + await PageObjects.visEditor.selectField('histogram-content', 'metrics'); + await PageObjects.visEditor.clickGo(); + + return await PageObjects.visChart.getTableVisContent(); + }; + + it('with percentiles aggregation', async () => { + const data = (await renderTableForAggregation('Percentiles')) as string[][]; + expect(data[0]).to.have.property('length', 7); + // Percentile values are not deterministic, so we can't check for the exact values here, + // but just check they are all within the given range + // see https://github.com/elastic/elasticsearch/issues/49225 + expect(data[0].every((p: string) => Number(p) >= 0.3 && Number(p) <= 5)).to.be(true); + }); + + it('with percentile ranks aggregation', async () => { + const data = await renderTableForAggregation('Percentile Ranks'); + expect(data).to.eql([['0%']]); + }); + + it('with average aggregation', async () => { + const data = await renderTableForAggregation('Average'); + expect(data).to.eql([['2.8510720308359434']]); + }); + + it('with mean aggregation', async () => { + // Percentile values (which are used by media behind the scenes) are not deterministic, + // so we can't check for the exact values here, but just check they are all within the given range + // see https://github.com/elastic/elasticsearch/issues/49225 + const data = await renderTableForAggregation('Median'); + const value = Number(data[0][0]); + expect(value).to.be.above(3.0); + expect(value).to.be.below(3.3); + }); - it('appears in the field options of a Percentile Ranks aggregation', async function() { - const fieldValues: string[] = await getFieldOptionsForAggregation('Percentile Ranks'); - log.debug('Percentile Ranks Fields = ' + fieldValues); - expect(fieldValues[0]).to.be('histogram-content'); + it('with sum aggregation', async () => { + const data = await renderTableForAggregation('Sum'); + expect(data).to.eql([['11834.800000000001']]); + }); }); }); } From 413cbbc718f37a0ef1efc8f21177ed4e417d91d8 Mon Sep 17 00:00:00 2001 From: Tim Roes Date: Tue, 19 May 2020 13:30:04 +0200 Subject: [PATCH 2/2] Fix PR review --- .../functional/apps/visualize/precalculated_histogram.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/x-pack/test/functional/apps/visualize/precalculated_histogram.ts b/x-pack/test/functional/apps/visualize/precalculated_histogram.ts index b012ca34b3ab1..d20af67508b57 100644 --- a/x-pack/test/functional/apps/visualize/precalculated_histogram.ts +++ b/x-pack/test/functional/apps/visualize/precalculated_histogram.ts @@ -39,9 +39,7 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { await PageObjects.visEditor.clickMetricEditor(); }); - const renderTableForAggregation = async ( - aggregation: string - ): Promise => { + const renderTableForAggregation = async (aggregation: string) => { await PageObjects.visEditor.selectAggregation(aggregation, 'metrics'); await PageObjects.visEditor.selectField('histogram-content', 'metrics'); await PageObjects.visEditor.clickGo(); @@ -68,8 +66,8 @@ export default function({ getService, getPageObjects }: FtrProviderContext) { expect(data).to.eql([['2.8510720308359434']]); }); - it('with mean aggregation', async () => { - // Percentile values (which are used by media behind the scenes) are not deterministic, + it('with median aggregation', async () => { + // Percentile values (which are used by median behind the scenes) are not deterministic, // so we can't check for the exact values here, but just check they are all within the given range // see https://github.com/elastic/elasticsearch/issues/49225 const data = await renderTableForAggregation('Median');