From 09c0952746d301c33ade12f661e10b4ffae7a413 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 19 May 2026 15:22:23 +0000 Subject: [PATCH 1/3] test: cover always-on distribution mode and color legend on Event Deltas The existing deltaChart*.test.ts files cover utility functions (applyTopNAggregation, getPropertyStatistics, filter-key mapping, scoring, sampling) but nothing renders DBDeltaChart or DBSearchHeatmapChart, so behaviours like "distribution mode renders without a heatmap selection" and "the parent passes legendPrefix" have no regression coverage. A bad merge-conflict resolution in the EE upstream merge for #1899 silently dropped both behaviours; the unit suite stayed green either way. Add two test files: - DBDeltaChart.test.tsx: distribution mode labels, selection mode labels, query gating (enabled flags for outlier / inlier / allSpans), the no-filters-override regression guard, and legendPrefix rendering. 9 tests. - Search/DBSearchHeatmapChart.test.tsx: asserts renders unconditionally (without URL selection coords), that URL coords are forwarded when set, and that the parent always passes a legendPrefix. 3 tests. All 12 pass on origin/main without any source change. They act as regression coverage so future merges that drop the always-on render path or the legendPrefix forwarding get caught at CI on both sides. Co-Authored-By: Claude Opus 4.1 --- .../__tests__/DBSearchHeatmapChart.test.tsx | 150 ++++++++++++++++++ .../__tests__/DBDeltaChart.test.tsx | 148 +++++++++++++++++ 2 files changed, 298 insertions(+) create mode 100644 packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx create mode 100644 packages/app/src/components/__tests__/DBDeltaChart.test.tsx diff --git a/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx new file mode 100644 index 0000000000..1b17e7633b --- /dev/null +++ b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx @@ -0,0 +1,150 @@ +import React from 'react'; +import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { screen } from '@testing-library/react'; + +import { DBSearchHeatmapChart } from '../DBSearchHeatmapChart'; + +// Controls the values returned by useQueryStates each render. +let mockFieldsState: Record = {}; + +jest.mock('nuqs', () => ({ + parseAsFloat: { withDefault: jest.fn(() => 'parseAsFloat') }, + parseAsString: { withDefault: jest.fn(() => 'parseAsString') }, + useQueryStates: jest.fn(() => [mockFieldsState, jest.fn()]), +})); + +jest.mock('@hyperdx/common-utils/dist/core/metadata', () => ({ + tcFromSource: jest.fn(() => ({})), +})); + +jest.mock('@/source', () => ({ + getDurationMsExpression: jest.fn(() => 'Duration'), +})); + +jest.mock('@/components/HeatmapSettingsDrawer', () => + jest.fn(() =>
), +); + +jest.mock('../../DBHeatmapChart', () => ({ + __esModule: true, + default: jest.fn((props: any) => ( +
+ )), + ColorLegend: jest.fn(() =>
color scale
), + darkPalette: ['#000'], + lightPalette: ['#fff'], + toHeatmapChartConfig: jest.fn(() => ({ heatmapConfig: {} })), +})); + +// Capture props passed to DBDeltaChart so we can assert URL coords forward. +const deltaChartPropsLog: any[] = []; +jest.mock('../../DBDeltaChart', () => ({ + __esModule: true, + default: jest.fn((props: any) => { + deltaChartPropsLog.push(props); + return
; + }), +})); + +const baseSource: any = { + id: 'trace-source', + kind: SourceKind.Trace, + name: 'Trace Source', + durationExpression: 'Duration', + spanIdExpression: 'SpanId', +}; + +const baseChartConfig: any = { + dateRange: [new Date(0), new Date(1000)], + from: { databaseName: 'otel', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + connection: 'conn', + select: '', + where: '', +}; + +describe('DBSearchHeatmapChart', () => { + beforeEach(() => { + jest.clearAllMocks(); + deltaChartPropsLog.length = 0; + mockFieldsState = { + value: 'Duration', + count: 'count()', + scaleType: 'log', + xMin: null, + xMax: null, + yMin: null, + yMax: null, + }; + }); + + it('renders DBDeltaChart unconditionally when no heatmap selection exists', () => { + // Regression test for #1899: the attribute distribution chart must + // render before any heatmap selection. A conditional wrapper here + // gates the always-on distribution mode and ships the feature off. + renderWithMantine( + , + ); + + expect(screen.getByTestId('db-delta-chart')).toBeInTheDocument(); + expect(deltaChartPropsLog.length).toBeGreaterThan(0); + expect(deltaChartPropsLog[deltaChartPropsLog.length - 1]).toMatchObject({ + xMin: null, + xMax: null, + yMin: null, + yMax: null, + valueExpr: 'Duration', + }); + }); + + it('forwards URL selection coordinates to DBDeltaChart', () => { + mockFieldsState = { + ...mockFieldsState, + xMin: 10, + xMax: 20, + yMin: 30, + yMax: 40, + }; + + renderWithMantine( + , + ); + + expect(screen.getByTestId('db-delta-chart')).toBeInTheDocument(); + expect(deltaChartPropsLog[deltaChartPropsLog.length - 1]).toMatchObject({ + xMin: 10, + xMax: 20, + yMin: 30, + yMax: 40, + }); + }); + + it('passes a ColorLegend as legendPrefix to DBDeltaChart', () => { + // Regression test: the parent must keep passing a ColorLegend as + // legendPrefix so the heatmap color scale appears above the + // distribution charts. DBDeltaChart renders this prop in its + // legend Flex. + renderWithMantine( + , + ); + + const lastProps = deltaChartPropsLog[deltaChartPropsLog.length - 1]; + expect(lastProps).toHaveProperty('legendPrefix'); + expect(lastProps.legendPrefix).toBeTruthy(); + }); +}); diff --git a/packages/app/src/components/__tests__/DBDeltaChart.test.tsx b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx new file mode 100644 index 0000000000..4b60633d2f --- /dev/null +++ b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx @@ -0,0 +1,148 @@ +import React from 'react'; +import { screen } from '@testing-library/react'; + +import { useQueriedChartConfig } from '@/hooks/useChartConfig'; + +import DBDeltaChart from '../DBDeltaChart'; + +jest.mock('@/hooks/useChartConfig', () => ({ + useQueriedChartConfig: jest.fn(), +})); + +// PropertyComparisonChart is the per-attribute renderer. Stubbing it out keeps +// the legend assertions tight without dragging uPlot/element-size hooks into +// the test environment. +jest.mock('../PropertyComparisonChart', () => ({ + __esModule: true, + CHART_GAP: 8, + CHART_HEIGHT: 100, + CHART_WIDTH: 200, + PAGINATION_HEIGHT: 32, + PropertyComparisonChart: ({ name }: { name: string }) => ( +
{name}
+ ), +})); + +const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; + +const baseConfig: any = { + dateRange: [new Date(0), new Date(1000)], + from: { databaseName: 'otel', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + connection: 'conn', + select: '', + where: '', + whereLanguage: 'sql' as const, +}; + +function renderChart( + overrides: Partial> = {}, +) { + return renderWithMantine( + , + ); +} + +describe('DBDeltaChart', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseQueriedChartConfig.mockReturnValue({ + data: { data: [], meta: [] }, + error: undefined, + isLoading: false, + }); + }); + + describe('distribution mode (no heatmap selection)', () => { + it('renders the "All spans" legend entry and the help hint', () => { + renderChart(); + + expect(screen.getByText('All spans')).toBeInTheDocument(); + expect( + screen.getByText( + 'Select an area on the chart above to enable comparisons', + ), + ).toBeInTheDocument(); + }); + + it('does not render the Selection/Background legend entries', () => { + renderChart(); + + expect(screen.queryByText('Selection')).not.toBeInTheDocument(); + expect(screen.queryByText('Background')).not.toBeInTheDocument(); + }); + + it('enables the allSpans query and disables the outlier/inlier queries', () => { + renderChart(); + + // The component fires three useQueriedChartConfig calls in order: + // outlier, inlier, allSpans. + const calls = mockUseQueriedChartConfig.mock.calls; + expect(calls).toHaveLength(3); + + const [outlierCall, inlierCall, allSpansCall] = calls; + expect(outlierCall[1]).toMatchObject({ enabled: false }); + expect(inlierCall[1]).toMatchObject({ enabled: false }); + expect(allSpansCall[1]).toMatchObject({ enabled: true }); + }); + + it('does not apply a filters override on the allSpans query', () => { + // Regression guard: the allSpans branch must not reuse buildFilters, + // which is keyed on (xMin/xMax/yMin/yMax). With no selection those + // default to 0 and produce dead SQL that mirrors the inlier branch. + renderChart(); + + const allSpansCall = mockUseQueriedChartConfig.mock.calls[2]; + const allSpansConfig = allSpansCall[0]; + expect(allSpansConfig.filters).toBeUndefined(); + }); + }); + + describe('selection mode (all four coordinates set)', () => { + it('renders the Selection and Background legend entries', () => { + renderChart({ xMin: 1, xMax: 2, yMin: 1, yMax: 2 }); + + expect(screen.getByText('Selection')).toBeInTheDocument(); + expect(screen.getByText('Background')).toBeInTheDocument(); + }); + + it('does not render the "All spans" legend entry', () => { + renderChart({ xMin: 1, xMax: 2, yMin: 1, yMax: 2 }); + + expect(screen.queryByText('All spans')).not.toBeInTheDocument(); + }); + + it('enables the outlier and inlier queries, disables allSpans', () => { + renderChart({ xMin: 1, xMax: 2, yMin: 1, yMax: 2 }); + + const calls = mockUseQueriedChartConfig.mock.calls; + const [outlierCall, inlierCall, allSpansCall] = calls; + expect(outlierCall[1]).toMatchObject({ enabled: true }); + expect(inlierCall[1]).toMatchObject({ enabled: true }); + expect(allSpansCall[1]).toMatchObject({ enabled: false }); + }); + }); + + describe('legendPrefix prop', () => { + it('renders inside the legend flex when provided', () => { + renderChart({ + legendPrefix:
color scale
, + }); + + expect(screen.getByTestId('prefix')).toBeInTheDocument(); + }); + + it('omits the divider when no legendPrefix is provided', () => { + // Without a prefix the divider should not render. We rely on the + // distribution-mode renderer above for shape; the absence assertion + // here is just "no prefix testid wrapper". + renderChart(); + expect(screen.queryByTestId('prefix')).not.toBeInTheDocument(); + }); + }); +}); From 0693145016a3cdc40b8afaa5c9ad25c01fdc1b33 Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 19 May 2026 15:40:41 +0000 Subject: [PATCH 2/3] test: tighten coverage per deep-review feedback - Add partial-null coordinate cases (one coord set, three coords set) so the four-null `&&` conjunction in `hasSelection` is locked in. A future regression weakening it to `||` would now fail at CI; previously the suite only exercised all-null and all-set. - Replace the placeholder divider absence test with an actual DOM check on the legend's vertical separator (`` with `borderLeft`), so removing the divider conditional would now fail. - Add `toHaveLength(3)` precondition before indexing `mockUseQueriedChartConfig.mock.calls[2]` so a missing allSpans call surfaces as a clear assertion failure, not an opaque "Cannot read properties of undefined". - Pin the `legendPrefix` element type to `ColorLegend` instead of truthiness; a swap to any other truthy node would now fail. - Type `deltaChartPropsLog` and `mockFieldsState` against the actual component prop / nuqs field shapes; remove `any` casts. - Extract `freshFieldsState()` so the module-level mutable state is reset from a single source of truth. --- .../__tests__/DBSearchHeatmapChart.test.tsx | 70 +++++++++++++------ .../__tests__/DBDeltaChart.test.tsx | 60 +++++++++++++--- 2 files changed, 100 insertions(+), 30 deletions(-) diff --git a/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx index 1b17e7633b..93acf070de 100644 --- a/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx +++ b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx @@ -1,11 +1,38 @@ import React from 'react'; -import { SourceKind } from '@hyperdx/common-utils/dist/types'; +import { + BuilderChartConfigWithDateRange, + SourceKind, + TTraceSource, +} from '@hyperdx/common-utils/dist/types'; import { screen } from '@testing-library/react'; +import type DBDeltaChartType from '../../DBDeltaChart'; import { DBSearchHeatmapChart } from '../DBSearchHeatmapChart'; +type FieldsState = { + value: string; + count: string; + scaleType: string; + xMin: number | null; + xMax: number | null; + yMin: number | null; + yMax: number | null; +}; + +function freshFieldsState(): FieldsState { + return { + value: 'Duration', + count: 'count()', + scaleType: 'log', + xMin: null, + xMax: null, + yMin: null, + yMax: null, + }; +} + // Controls the values returned by useQueryStates each render. -let mockFieldsState: Record = {}; +let mockFieldsState: FieldsState = freshFieldsState(); jest.mock('nuqs', () => ({ parseAsFloat: { withDefault: jest.fn(() => 'parseAsFloat') }, @@ -27,7 +54,7 @@ jest.mock('@/components/HeatmapSettingsDrawer', () => jest.mock('../../DBHeatmapChart', () => ({ __esModule: true, - default: jest.fn((props: any) => ( + default: jest.fn((props: { enabled?: boolean }) => (
({ })); // Capture props passed to DBDeltaChart so we can assert URL coords forward. -const deltaChartPropsLog: any[] = []; +type DBDeltaChartProps = React.ComponentProps; +const deltaChartPropsLog: DBDeltaChartProps[] = []; jest.mock('../../DBDeltaChart', () => ({ __esModule: true, - default: jest.fn((props: any) => { + default: jest.fn((props: DBDeltaChartProps) => { deltaChartPropsLog.push(props); return
; }), })); -const baseSource: any = { +// Re-import the mocked ColorLegend so we can assert that it's specifically +// the heatmap color scale being passed as legendPrefix (not any truthy node). +const { ColorLegend: MockedColorLegend } = jest.requireMock( + '../../DBHeatmapChart', +); + +const baseSource = { id: 'trace-source', kind: SourceKind.Trace, name: 'Trace Source', durationExpression: 'Duration', spanIdExpression: 'SpanId', -}; +} as unknown as TTraceSource; -const baseChartConfig: any = { +const baseChartConfig = { dateRange: [new Date(0), new Date(1000)], from: { databaseName: 'otel', tableName: 'otel_traces' }, timestampValueExpression: 'Timestamp', connection: 'conn', select: '', where: '', -}; +} as unknown as BuilderChartConfigWithDateRange; describe('DBSearchHeatmapChart', () => { beforeEach(() => { jest.clearAllMocks(); deltaChartPropsLog.length = 0; - mockFieldsState = { - value: 'Duration', - count: 'count()', - scaleType: 'log', - xMin: null, - xMax: null, - yMin: null, - yMax: null, - }; + mockFieldsState = freshFieldsState(); }); it('renders DBDeltaChart unconditionally when no heatmap selection exists', () => { @@ -134,7 +160,8 @@ describe('DBSearchHeatmapChart', () => { // Regression test: the parent must keep passing a ColorLegend as // legendPrefix so the heatmap color scale appears above the // distribution charts. DBDeltaChart renders this prop in its - // legend Flex. + // legend Flex. Truthiness alone would not catch a regression that + // swaps ColorLegend for any other element; pin the element type. renderWithMantine( { ); const lastProps = deltaChartPropsLog[deltaChartPropsLog.length - 1]; - expect(lastProps).toHaveProperty('legendPrefix'); expect(lastProps.legendPrefix).toBeTruthy(); + expect(React.isValidElement(lastProps.legendPrefix)).toBe(true); + expect((lastProps.legendPrefix as React.ReactElement).type).toBe( + MockedColorLegend, + ); }); }); diff --git a/packages/app/src/components/__tests__/DBDeltaChart.test.tsx b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx index 4b60633d2f..300935456f 100644 --- a/packages/app/src/components/__tests__/DBDeltaChart.test.tsx +++ b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx @@ -1,4 +1,5 @@ import React from 'react'; +import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; import { screen } from '@testing-library/react'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; @@ -25,14 +26,14 @@ jest.mock('../PropertyComparisonChart', () => ({ const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; -const baseConfig: any = { +const baseConfig: BuilderChartConfigWithDateRange = { dateRange: [new Date(0), new Date(1000)], from: { databaseName: 'otel', tableName: 'otel_traces' }, timestampValueExpression: 'Timestamp', connection: 'conn', select: '', where: '', - whereLanguage: 'sql' as const, + whereLanguage: 'sql', }; function renderChart( @@ -97,12 +98,39 @@ describe('DBDeltaChart', () => { // default to 0 and produce dead SQL that mirrors the inlier branch. renderChart(); - const allSpansCall = mockUseQueriedChartConfig.mock.calls[2]; - const allSpansConfig = allSpansCall[0]; + const calls = mockUseQueriedChartConfig.mock.calls; + expect(calls).toHaveLength(3); + const allSpansConfig = calls[2][0]; expect(allSpansConfig.filters).toBeUndefined(); }); }); + describe('partial-null selection coordinates', () => { + // The four-null conditional that originally gated + // used `&&` across xMin/xMax/yMin/yMax. A regression weakening the + // conjunction to `||` would let selection mode activate with any + // single coord set, which is wrong. These cases lock the conjunction + // in: any null coord must keep distribution mode active. + it.each([ + ['only xMin set', { xMin: 1 }], + ['only xMax set', { xMax: 2 }], + ['only yMin set', { yMin: 1 }], + ['only yMax set', { yMax: 2 }], + ['three set, yMax null', { xMin: 1, xMax: 2, yMin: 1 }], + ['three set, xMin null', { xMax: 2, yMin: 1, yMax: 2 }], + ])('stays in distribution mode when %s', (_label, coords) => { + renderChart(coords); + + expect(screen.getByText('All spans')).toBeInTheDocument(); + expect(screen.queryByText('Selection')).not.toBeInTheDocument(); + + const calls = mockUseQueriedChartConfig.mock.calls; + expect(calls[2][1]).toMatchObject({ enabled: true }); + expect(calls[0][1]).toMatchObject({ enabled: false }); + expect(calls[1][1]).toMatchObject({ enabled: false }); + }); + }); + describe('selection mode (all four coordinates set)', () => { it('renders the Selection and Background legend entries', () => { renderChart({ xMin: 1, xMax: 2, yMin: 1, yMax: 2 }); @@ -129,20 +157,32 @@ describe('DBDeltaChart', () => { }); describe('legendPrefix prop', () => { + // The divider is a vertical separator rendered immediately after the + // legendPrefix to visually divide it from the comparison legend. It's + // a `` with a `borderLeft` style. We assert its presence + // via the unique border style so the test is decoupled from Mantine's + // class hashing. + const dividerStyle = '1px solid var(--mantine-color-default-border)'; + + function queryDivider(container: HTMLElement) { + return Array.from(container.querySelectorAll('div')).find( + el => el.style.borderLeft === dividerStyle, + ); + } + it('renders inside the legend flex when provided', () => { - renderChart({ + const { container } = renderChart({ legendPrefix:
color scale
, }); expect(screen.getByTestId('prefix')).toBeInTheDocument(); + expect(queryDivider(container)).toBeDefined(); }); - it('omits the divider when no legendPrefix is provided', () => { - // Without a prefix the divider should not render. We rely on the - // distribution-mode renderer above for shape; the absence assertion - // here is just "no prefix testid wrapper". - renderChart(); + it('omits the prefix and divider when no legendPrefix is provided', () => { + const { container } = renderChart(); expect(screen.queryByTestId('prefix')).not.toBeInTheDocument(); + expect(queryDivider(container)).toBeUndefined(); }); }); }); From c54b384e83fb041c4487fcdffdfb5656b2a0d0ba Mon Sep 17 00:00:00 2001 From: Alex Fedotyev <61838744+alex-fedotyev@users.noreply.github.com> Date: Tue, 19 May 2026 17:23:21 +0000 Subject: [PATCH 3/3] test: drop fixture double casts and tighten allSpans filters guard Two follow-ups from the deep-review pass on this PR: 1. baseSource and baseChartConfig in DBSearchHeatmapChart.test.tsx were each created with `as unknown as ` double casts, which silenced TypeScript on missing required fields. Replaced with direct type annotations against TTraceSource and BuilderChartConfigWithDateRange, filling in every required slot (durationPrecision, traceIdExpression, parentSpanIdExpression, spanNameExpression, spanKindExpression, defaultTableSelectExpression, whereLanguage). Adding a new required field to either schema now fails this fixture at compile time instead of slipping through. 2. The allSpans "no filters override" assertion previously checked `allSpansConfig.filters` is undefined, but baseConfig had no filters either, so the assertion passed both when allSpans correctly omits selection-keyed filters AND when it silently drops user-supplied filters on the floor. Added a sentinel `filters: [{ type: 'sql', condition: 'tenant_id = 42' }]` to baseConfig and assert allSpans receives exactly that array (toEqual), plus a sanity check that outlier filters are strictly longer and start with the same sentinel. Now catches three failure modes: allSpans reusing buildFilters (length grows), allSpans explicitly clearing filters (length drops), or buildFilters losing the config.filters spread on outlier. All 18 tests still pass. Typecheck error count is unchanged. --- .../__tests__/DBSearchHeatmapChart.test.tsx | 21 +++++++++-- .../__tests__/DBDeltaChart.test.tsx | 37 ++++++++++++++++--- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx index 93acf070de..60223674c7 100644 --- a/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx +++ b/packages/app/src/components/Search/__tests__/DBSearchHeatmapChart.test.tsx @@ -83,22 +83,35 @@ const { ColorLegend: MockedColorLegend } = jest.requireMock( '../../DBHeatmapChart', ); -const baseSource = { +// Direct type annotations (no `as unknown as` casts) so adding a +// required field to either schema fails this fixture at compile +// time rather than silently passing. +const baseSource: TTraceSource = { id: 'trace-source', kind: SourceKind.Trace, name: 'Trace Source', + connection: 'conn', + from: { databaseName: 'otel', tableName: 'otel_traces' }, + timestampValueExpression: 'Timestamp', + defaultTableSelectExpression: 'Timestamp', durationExpression: 'Duration', + durationPrecision: 3, + traceIdExpression: 'TraceId', spanIdExpression: 'SpanId', -} as unknown as TTraceSource; + parentSpanIdExpression: 'ParentSpanId', + spanNameExpression: 'SpanName', + spanKindExpression: 'SpanKind', +}; -const baseChartConfig = { +const baseChartConfig: BuilderChartConfigWithDateRange = { dateRange: [new Date(0), new Date(1000)], from: { databaseName: 'otel', tableName: 'otel_traces' }, timestampValueExpression: 'Timestamp', connection: 'conn', select: '', where: '', -} as unknown as BuilderChartConfigWithDateRange; + whereLanguage: 'sql', +}; describe('DBSearchHeatmapChart', () => { beforeEach(() => { diff --git a/packages/app/src/components/__tests__/DBDeltaChart.test.tsx b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx index 300935456f..328805fbe1 100644 --- a/packages/app/src/components/__tests__/DBDeltaChart.test.tsx +++ b/packages/app/src/components/__tests__/DBDeltaChart.test.tsx @@ -1,5 +1,8 @@ import React from 'react'; -import { BuilderChartConfigWithDateRange } from '@hyperdx/common-utils/dist/types'; +import { + BuilderChartConfigWithDateRange, + Filter, +} from '@hyperdx/common-utils/dist/types'; import { screen } from '@testing-library/react'; import { useQueriedChartConfig } from '@/hooks/useChartConfig'; @@ -26,6 +29,15 @@ jest.mock('../PropertyComparisonChart', () => ({ const mockUseQueriedChartConfig = useQueriedChartConfig as jest.Mock; +// Sentinel filter on baseConfig so the allSpans assertion can prove +// the branch passes user-supplied filters through unchanged. Without +// it, `expect(filters).toBeUndefined()` passes for both "no override" +// and "filters dropped on the floor", which are indistinguishable. +const sentinelFilter: Filter = { + type: 'sql', + condition: 'tenant_id = 42', +}; + const baseConfig: BuilderChartConfigWithDateRange = { dateRange: [new Date(0), new Date(1000)], from: { databaseName: 'otel', tableName: 'otel_traces' }, @@ -34,6 +46,7 @@ const baseConfig: BuilderChartConfigWithDateRange = { select: '', where: '', whereLanguage: 'sql', + filters: [sentinelFilter], }; function renderChart( @@ -92,16 +105,28 @@ describe('DBDeltaChart', () => { expect(allSpansCall[1]).toMatchObject({ enabled: true }); }); - it('does not apply a filters override on the allSpans query', () => { - // Regression guard: the allSpans branch must not reuse buildFilters, - // which is keyed on (xMin/xMax/yMin/yMax). With no selection those - // default to 0 and produce dead SQL that mirrors the inlier branch. + it('passes config.filters through to allSpans without selection filters', () => { + // Regression guard for two related failure modes: + // 1. allSpans reuses buildFilters() (keyed on xMin/xMax/yMin/yMax). + // With no selection those default to 0 and produce dead SQL + // mirroring the inlier branch. Length would grow past 1. + // 2. allSpans explicitly sets `filters: undefined` and drops the + // user-supplied filters on the floor. Length would drop to 0. + // Both are caught by asserting allSpans receives exactly the + // sentinel filter array from config, no more and no fewer. renderChart(); const calls = mockUseQueriedChartConfig.mock.calls; expect(calls).toHaveLength(3); const allSpansConfig = calls[2][0]; - expect(allSpansConfig.filters).toBeUndefined(); + expect(allSpansConfig.filters).toEqual([sentinelFilter]); + + // Sanity: the outlier branch DOES inject extra filters built from + // the (defaulted-to-zero) coords, so its filters array is strictly + // longer than baseConfig.filters. + const outlierConfig = calls[0][0]; + expect(outlierConfig.filters.length).toBeGreaterThan(1); + expect(outlierConfig.filters[0]).toEqual(sentinelFilter); }); });