From 5b5f23d12727805407571a4e89172a6e9883d913 Mon Sep 17 00:00:00 2001 From: jesperct Date: Tue, 5 May 2026 12:37:35 -0300 Subject: [PATCH] test(plugin-chart-echarts): regression guards for temporal x-axis labels on timeseries charts (#39208) --- .../MixedTimeseries/transformProps.test.ts | 132 ++++++++++++++++++ .../test/Timeseries/transformProps.test.ts | 113 +++++++++++++++ .../test/utils/formatters.test.ts | 52 +++++++ .../test/utils/series.test.ts | 16 +++ 4 files changed, 313 insertions(+) diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts index 67ee4500bb8..7d9deaa7a69 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts @@ -20,12 +20,14 @@ import { AnnotationStyle, AnnotationType, AnnotationSourceType, + AxisType, DataRecord, FormulaAnnotationLayer, IntervalAnnotationLayer, VizType, ChartDataResponseResult, } from '@superset-ui/core'; +import { GenericDataType } from '@apache-superset/core/common'; import { LegendOrientation, LegendType, @@ -496,3 +498,133 @@ test('should add a formula annotation when X-axis column has dataset-level label expect(Array.isArray(formulaSeries?.data)).toBe(true); expect((formulaSeries!.data as unknown[]).length).toBeGreaterThan(0); }); + +test('numeric x coltype never gets silently coerced to the Time axis', () => { + // Regression guard for echarts-timeseries-epoch-x-axis-labels investigation. + // Mixed Timeseries must follow the reported coltype: Numeric values stay + // off the Time axis and are not silently reinterpreted as Date instances. + // A future change that coerces Numeric → Time would bring back the "NaN" + // label symptom we were investigating. We also assert that whichever + // formatter is picked, it produces a string and does not emit "NaN". + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const epochRows = [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ]; + const epochQueryData = createTestQueryData(epochRows, { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + label_map: { __timestamp: ['__timestamp'], metric: ['metric'] }, + }); + + const chartProps = createEchartsTimeseriesTestChartProps< + EchartsMixedTimeseriesFormData, + EchartsMixedTimeseriesProps + >({ + ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS, + defaultQueriesData: [epochQueryData, epochQueryData], + formData: { + ...formData, + x_axis: '__timestamp', + metrics: ['metric'], + metricsB: ['metric'], + groupby: [], + groupbyB: [], + }, + queriesData: [epochQueryData, epochQueryData], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { + type: string; + axisLabel: { formatter: (v: number) => string }; + }; + + expect(xAxis.type).not.toBe(AxisType.Time); + const label = xAxis.axisLabel.formatter(ts1); + expect(typeof label).toBe('string'); + expect(label).not.toMatch(/NaN/); +}); + +test('xAxisForceCategorical forces Category axis regardless of Numeric coltype', () => { + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const epochRows = [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ]; + const epochQueryData = createTestQueryData(epochRows, { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + label_map: { __timestamp: ['__timestamp'], metric: ['metric'] }, + }); + + const chartProps = createEchartsTimeseriesTestChartProps< + EchartsMixedTimeseriesFormData, + EchartsMixedTimeseriesProps + >({ + ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS, + defaultQueriesData: [epochQueryData, epochQueryData], + formData: { + ...formData, + x_axis: '__timestamp', + metrics: ['metric'], + metricsB: ['metric'], + groupby: [], + groupbyB: [], + xAxisForceCategorical: true, + }, + queriesData: [epochQueryData, epochQueryData], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { type: string }; + + expect(xAxis.type).toBe(AxisType.Category); +}); + +test('temporal x coltype wires the time formatter and Time axis', () => { + // Regression guard: the happy path for mixed-timeseries charts. Ensures + // Temporal coltype still routes through the TimeFormatter so the time axis + // rendering path is exercised by the test suite. + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const temporalRows = [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ]; + const temporalQueryData = createTestQueryData(temporalRows, { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Temporal, GenericDataType.Numeric], + label_map: { __timestamp: ['__timestamp'], metric: ['metric'] }, + }); + + const chartProps = createEchartsTimeseriesTestChartProps< + EchartsMixedTimeseriesFormData, + EchartsMixedTimeseriesProps + >({ + ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS, + defaultQueriesData: [temporalQueryData, temporalQueryData], + formData: { + ...formData, + x_axis: '__timestamp', + metrics: ['metric'], + metricsB: ['metric'], + groupby: [], + groupbyB: [], + }, + queriesData: [temporalQueryData, temporalQueryData], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { + type: string; + axisLabel: { formatter: (v: Date) => string }; + }; + + expect(xAxis.type).toBe(AxisType.Time); + const label = xAxis.axisLabel.formatter(new Date(ts1)); + expect(typeof label).toBe('string'); + expect(label).not.toMatch(/NaN/); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts index 373cf6a0d54..a43610f3348 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts @@ -20,6 +20,7 @@ import { AnnotationSourceType, AnnotationStyle, AnnotationType, + AxisType, ComparisonType, DataRecord, EventAnnotationLayer, @@ -1472,6 +1473,118 @@ test('x-axis formatter deduplicates consecutive identical labels for coarse time expect(label4).toBe(''); }); +test('numeric x coltype routes through the number formatter (not the time formatter)', () => { + // Regression guard for echarts-timeseries-epoch-x-axis-labels investigation. + // When the query reports a Numeric x-axis coltype (including epoch-ms-like + // values), Timeseries transformProps must pick the Value axis and run the + // label through getNumberFormatter, not the time formatter. If this ever + // changes, epoch-ms values that arrive as Numeric would suddenly be treated + // as Date instances and could render "NaN" — the symptom that prompted this + // investigation. + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const chartProps = createTestChartProps({ + formData: { + metrics: ['metric'], + granularity_sqla: 'ds', + x_axis: '__timestamp', + }, + queriesData: [ + createTestQueryData( + [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ], + { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + ), + ], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { + type: string; + axisLabel: { formatter: (v: number) => string }; + }; + + expect(xAxis.type).toBe(AxisType.Value); + const label = xAxis.axisLabel.formatter(ts1); + expect(typeof label).toBe('string'); + expect(label).not.toMatch(/NaN/); +}); + +test('xAxisForceCategorical forces Category axis regardless of Numeric coltype', () => { + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const chartProps = createTestChartProps({ + formData: { + metrics: ['metric'], + granularity_sqla: 'ds', + x_axis: '__timestamp', + xAxisForceCategorical: true, + }, + queriesData: [ + createTestQueryData( + [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ], + { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Numeric, GenericDataType.Numeric], + }, + ), + ], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { type: string }; + + expect(xAxis.type).toBe(AxisType.Category); +}); + +test('temporal x coltype wires the time formatter and Time axis', () => { + // Regression guard: the happy path for time-series charts. Ensures that + // Temporal coltype keeps routing through the TimeFormatter so a refactor + // does not accidentally drop Date handling (the feared regression that + // sparked this investigation). + const ts1 = 1745784000000; + const ts2 = 1745870400000; + const chartProps = createTestChartProps({ + formData: { + metrics: ['metric'], + granularity_sqla: 'ds', + x_axis: '__timestamp', + }, + queriesData: [ + createTestQueryData( + [ + { __timestamp: ts1, metric: 10 }, + { __timestamp: ts2, metric: 20 }, + ], + { + colnames: ['__timestamp', 'metric'], + coltypes: [GenericDataType.Temporal, GenericDataType.Numeric], + }, + ), + ], + }); + + const { echartOptions } = transformProps(chartProps); + const xAxis = echartOptions.xAxis as { + type: string; + axisLabel: { formatter: (v: Date) => string }; + }; + + expect(xAxis.type).toBe(AxisType.Time); + const label = xAxis.axisLabel.formatter(new Date(ts1)); + expect(typeof label).toBe('string'); + expect(label).not.toMatch(/NaN/); + expect(label).not.toBe(String(ts1)); +}); + test('should assign distinct dash patterns for multiple time offsets consistently', () => { const queriesDataWithMultipleOffsets = [ createTestQueryData([ diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts index a4e4dea5a4b..d60c6308717 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/formatters.test.ts @@ -19,11 +19,13 @@ import { NumberFormats, SMART_DATE_ID, + SMART_DATE_VERBOSE_ID, TimeFormatter, TimeGranularity, } from '@superset-ui/core'; import { getPercentFormatter, + getTooltipTimeFormatter, getXAxisFormatter, } from '../../src/utils/formatters'; @@ -179,3 +181,53 @@ test('getXAxisFormatter without time grain should use standard smart date behavi expect(standardResult).toBe(timeGrainResult); }); + +// Regression tests for echarts-timeseries-epoch-x-axis-labels investigation. +// The bug report was that temporal x-axis labels could render as "NaN" +// in some edge cases that we could not reproduce locally. The tests below +// lock in the current behavior of the formatters so that a future refactor +// surfaces any change in contract. + +test('getTooltipTimeFormatter returns a TimeFormatter with SMART_DATE_VERBOSE id for SMART_DATE_ID', () => { + const formatter = getTooltipTimeFormatter(SMART_DATE_ID); + expect(formatter).toBeInstanceOf(TimeFormatter); + expect((formatter as TimeFormatter).id).toBe(SMART_DATE_VERBOSE_ID); +}); + +test('getTooltipTimeFormatter returns a TimeFormatter for a custom format string', () => { + const customFormat = '%Y-%m-%d %H:%M'; + const formatter = getTooltipTimeFormatter(customFormat); + expect(formatter).toBeInstanceOf(TimeFormatter); + expect((formatter as TimeFormatter).id).toBe(customFormat); +}); + +test('getTooltipTimeFormatter falls back to the String constructor when no format is supplied', () => { + expect(getTooltipTimeFormatter()).toBe(String); + expect(getTooltipTimeFormatter(undefined)).toBe(String); +}); + +test('getXAxisFormatter produces stable SMART_DATE output for a valid Date', () => { + // Documents the current happy-path output format so unexpected changes are + // caught during review. + const formatter = getXAxisFormatter(SMART_DATE_ID) as TimeFormatter; + const result = formatter.format(new Date('2025-01-15T00:00:00.000Z')); + expect(typeof result).toBe('string'); + expect(result).not.toMatch(/NaN/); + expect(result.length).toBeGreaterThan(0); +}); + +test('getXAxisFormatter returns a string for an Invalid Date without throwing', () => { + // If a caller ever passes an Invalid Date (the originally-suspected cause + // of epoch-ms axis labels showing NaN in echarts), the formatter must + // still return a string instead of throwing, so echarts does not blow up + // the chart render. The *content* of that string is format-dependent and + // intentionally not asserted here — only that it is a string. + const formatter = getXAxisFormatter(SMART_DATE_ID) as TimeFormatter; + const invalid = new Date(Number.NaN); + expect(() => formatter.format(invalid)).not.toThrow(); + expect(typeof formatter.format(invalid)).toBe('string'); + + const customFormatter = getXAxisFormatter('%Y-%m-%d') as TimeFormatter; + expect(() => customFormatter.format(invalid)).not.toThrow(); + expect(typeof customFormatter.format(invalid)).toBe('string'); +}); diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts index 5bf971229e7..fad7944f6a5 100644 --- a/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts +++ b/superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts @@ -1419,6 +1419,22 @@ test('getAxisType treats numeric as category for bar charts', () => { ).toEqual(AxisType.Value); }); +test('getAxisType does not coerce Numeric x-axis to Time regardless of values', () => { + // Regression guard for echarts-timeseries-epoch-x-axis-labels investigation: + // getAxisType only considers the coltype reported by the query, never the + // actual values. Numeric coltype must stay on a Value axis so a future + // change that introduces implicit temporal coercion is surfaced here. + expect(getAxisType(false, false, GenericDataType.Numeric)).toEqual( + AxisType.Value, + ); + expect(getAxisType(false, false, GenericDataType.Temporal)).toEqual( + AxisType.Time, + ); + expect(getAxisType(false, false, GenericDataType.String)).toEqual( + AxisType.Category, + ); +}); + test('getMinAndMaxFromBounds returns empty object when not truncating', () => { expect( getMinAndMaxFromBounds(