bito-code-review[bot] commented on code in PR #43339:
URL: https://github.com/apache/superset/pull/43339#discussion_r3880724530
##########
superset-frontend/plugins/plugin-chart-echarts/test/utils/series.test.ts:
##########
@@ -1705,6 +1708,129 @@ test('getAxisType does not coerce Numeric x-axis to
Time regardless of values',
);
});
+describe('getTemporalTickValues', () => {
+ const xAxisLabel = '__timestamp';
+
+ test('returns undefined for a non-time axis', () => {
+ const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Category,
+ TimeGranularity.WEEK,
+ ),
+ ).toBeUndefined();
+ });
+
+ test('returns undefined when there is no time grain', () => {
+ const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
+ expect(
+ getTemporalTickValues(data, xAxisLabel, AxisType.Time, undefined),
+ ).toBeUndefined();
+ });
+
+ test('returns undefined for a non-weekly time grain', () => {
+ const data: DataRecord[] = [{ [xAxisLabel]: 1712361600000 }];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.MONTH,
+ ),
+ ).toBeUndefined();
+ });
+
+ test('returns sorted, de-duplicated bucket timestamps for numbers and
Dates', () => {
+ const t0 = Date.UTC(2026, 3, 6);
+ const t1 = Date.UTC(2026, 3, 13);
+ const data: DataRecord[] = [
+ { [xAxisLabel]: t1 },
+ { [xAxisLabel]: new Date(t0) },
+ { [xAxisLabel]: t0 }, // duplicate of the Date row above
+ ];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.WEEK,
+ ),
+ ).toEqual([t0, t1]);
+ });
+
+ test('parses a zoned ISO string as the instant it names', () => {
+ const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06T00:00:00.000Z' }];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.WEEK,
+ ),
+ ).toEqual([Date.UTC(2026, 3, 6)]);
+ });
+
+ test('parses a zone-less datetime string as local time, matching ECharts',
() => {
+ const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06T00:00:00' }];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.WEEK,
+ ),
+ ).toEqual([new Date(2026, 3, 6, 0, 0, 0).getTime()]);
+ });
+
+ test('parses a bare date string as local midnight, matching ECharts rather
than native Date', () => {
+ // `new Date('2026-04-06')` is UTC, but ECharts parses it as local time.
+ // jest.config.js fixes the test TZ to America/New_York, so they disagree.
+ const data: DataRecord[] = [{ [xAxisLabel]: '2026-04-06' }];
+ const localMidnight = new Date(2026, 3, 6).getTime();
+ expect(localMidnight).not.toEqual(new Date('2026-04-06').getTime());
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.WEEK,
+ ),
+ ).toEqual([localMidnight]);
+ });
+
+ test('drops unparseable or nullish values and returns undefined when none
remain', () => {
+ const data: DataRecord[] = [
+ { [xAxisLabel]: 'not-a-date' },
+ { [xAxisLabel]: null },
+ ];
+ expect(
+ getTemporalTickValues(
+ data,
+ xAxisLabel,
+ AxisType.Time,
+ TimeGranularity.WEEK,
+ ),
+ ).toBeUndefined();
+ });
+});
+
+describe('capTickMarks', () => {
+ test('returns values unchanged when within the cap', () => {
+ const values = [1, 2, 3];
+ expect(capTickMarks(values, 60)).toEqual(values);
+ });
+
+ test('downsamples evenly and always keeps the last value', () => {
+ const values = Array.from({ length: 261 }, (_, i) => i);
+ const capped = capTickMarks(values, 60);
+ expect(capped.length).toBeLessThanOrEqual(60);
+ expect(capped[0]).toEqual(0);
+ expect(capped[capped.length - 1]).toEqual(260);
+ });
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Weak test assertions on downsampling</b></div>
<div id="fix">
Test claims to verify 'downsamples evenly' but only checks `length <= 60`,
first, and last. An implementation that returned `[0, 260]` would still pass.
Per rule [6262], assert the actual downsampling logic in `capTickMarks` — e.g.,
`expect(capped.length).toBe(53)` for `Array.from({length:261})` with cap 60, or
check that consecutive entries differ by the step.
</div>
</div>
<small><i>Code Review Run #4f4d6f</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
