codeant-ai-for-open-source[bot] commented on code in PR #36993:
URL: https://github.com/apache/superset/pull/36993#discussion_r2692705496
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts:
##########
@@ -19,10 +19,14 @@
import { CategoricalColorScale } from '@superset-ui/core';
import type { SeriesOption } from 'echarts';
import { EchartsTimeseriesSeriesType } from '../../src';
+import { TIMESERIES_CONSTANTS } from '../../src/constants';
+import { LegendOrientation } from '../../src/types';
import {
transformSeries,
transformNegativeLabelsPosition,
+ getPadding,
} from '../../src/Timeseries/transformers';
Review Comment:
**Suggestion:** The test imports `getPadding` from
'../../src/Timeseries/transformers' but that function is not exported from that
module (the real helper that is being spied on is `getChartPadding` in
'../../src/utils/series'). Importing the wrong symbol will cause the test
module to fail to load or call an undefined function; replace the import so
tests call the actual util exported from the utils module. [possible bug]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Jest tests fail loading this test file.
- ❌ CI job for chart-echarts tests will error.
- ⚠️ Blocks PR validation for related chart plugin.
```
</details>
```suggestion
} from '../../src/Timeseries/transformers';
import { getChartPadding as getPadding } from '../../src/utils/series';
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the test file imports at
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
lines
22-29 where getPadding is imported from ../../src/Timeseries/transformers
(see line 27 in
file). The test expects a function named getPadding to be present.
2. Jest starts loading the test module and resolves imports. Jest will
attempt to import
getPadding from ../../src/Timeseries/transformers at module load time
(import occurs at
file top, lines 22-29).
3. The actual transformers module (src/Timeseries/transformers.ts) in the
final code
exports transformSeries and transformNegativeLabelsPosition but contains no
exported
getPadding function (inspect transformers.ts: transformSeries is defined
starting around
line 167; no getPadding export exists).
4. Because getPadding is not exported from transformers, the test module
will either
receive undefined for getPadding or the import will fail to resolve the
symbol, causing
the test file to error when it calls getPadding (call sites are at
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts:229
where getPadding(...) is invoked). This triggers an immediate Jest load-time
failure for
this test file.
5. Replace the import to pull getChartPadding from src/utils/series (the
test already
spies on seriesUtils.getChartPadding), or re-export a proper getPadding
wrapper from
transformers if intentional. The improved import maps getChartPadding to
getPadding so
call sites continue to work with the spy.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
**Line:** 27:28
**Comment:**
*Possible Bug: The test imports `getPadding` from
'../../src/Timeseries/transformers' but that function is not exported from that
module (the real helper that is being spied on is `getChartPadding` in
'../../src/utils/series'). Importing the wrong symbol will cause the test
module to fail to load or call an undefined function; replace the import so
tests call the actual util exported from the utils module.
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>
--
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]
