codeant-ai-for-open-source[bot] commented on code in PR #38949:
URL: https://github.com/apache/superset/pull/38949#discussion_r3009141915
##########
superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts:
##########
@@ -67,6 +67,11 @@ describe('TimeFormatter', () => {
test('handles number, treating it as a timestamp', () => {
expect(formatter.format(PREVIEW_TIME.getTime())).toEqual('2017');
});
+ test('handles numeric string, treating it as a timestamp', () => {
+ // PivotData.processRecord coerces values with String(), turning numeric
+ // timestamps into strings. 1704067200000 = 2024-01-01T00:00:00.000Z
+ expect(formatter.format('1704067200000')).toEqual('2024');
Review Comment:
**Suggestion:** The new test hardcodes a timestamp representing midnight UTC
on Jan 1, 2024, but because `getFullYear()` uses the local timezone, in
negative-offset timezones this timestamp falls on Dec 31, 2023 locally, causing
the test to intermittently fail depending on the environment; use a timestamp
derived from `PREVIEW_TIME` (or another date safely away from year boundaries)
so the expected year is stable in all timezones. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ TimeFormatter unit test fails on some developer machines.
- ⚠️ CI may intermittently fail if not forced to UTC.
- ⚠️ Flaky timezone-dependent test reduces trust in test suite.
```
</details>
```suggestion
// timestamps into strings.
const timestamp = PREVIEW_TIME.getTime().toString();
expect(formatter.format(timestamp)).toEqual('2017');
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. On a machine whose local timezone is behind UTC (e.g., America/New_York),
run the Jest
test suite that includes
`superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts`.
2. In `.format(value)` tests at `TimeFormatter.test.ts:56-77`, the test
`handles numeric
string, treating it as a timestamp` at lines 70-73 calls
`formatter.format('1704067200000')`, with `formatter` created above using
`formatFunc:
value => \`\${value.getFullYear()}\``.
3. `TimeFormatter.format` at `src/time-format/TimeFormatter.ts:71-73`
delegates to
`stringifyTimeInput` in `src/time-format/utils/stringifyTimeInput.ts:20-33`,
which detects
a string input, converts it with `Number(value)`, and creates `new
Date(1704067200000)`.
4. In a negative-offset timezone, `new Date(1704067200000)` represents local
time late on
2023-12-31, so `getFullYear()` in the test's `formatFunc` returns `2023`,
while the test
at `TimeFormatter.test.ts:73` expects `'2024'`, causing the test to fail on
such
environments but pass in UTC or positive-offset timezones.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/packages/superset-ui-core/test/time-format/TimeFormatter.test.ts
**Line:** 72:73
**Comment:**
*Logic Error: The new test hardcodes a timestamp representing midnight
UTC on Jan 1, 2024, but because `getFullYear()` uses the local timezone, in
negative-offset timezones this timestamp falls on Dec 31, 2023 locally, causing
the test to intermittently fail depending on the environment; use a timestamp
derived from `PREVIEW_TIME` (or another date safely away from year boundaries)
so the expected year is stable in all timezones.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38949&comment_hash=842ac2c96e8bbe1f064486898de370d9c34f0a69e0e59cb991ec31e010dc035c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38949&comment_hash=842ac2c96e8bbe1f064486898de370d9c34f0a69e0e59cb991ec31e010dc035c&reaction=dislike'>👎</a>
--
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]
