codeant-ai-for-open-source[bot] commented on code in PR #37225:
URL: https://github.com/apache/superset/pull/37225#discussion_r2764160123
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx:
##########
@@ -45,7 +45,7 @@ jest.mock('../components/Echart', () => {
const MockEchart = forwardRef<EchartsHandler | null, EchartsProps>(
(props, ref) => {
mockEchart(props);
- void ref;
+ console.log(ref);
Review Comment:
**Suggestion:** The `console.log(ref);` call inside the mocked `Echart`
component is a leftover debug side effect that will run on every test render,
cluttering test output and potentially breaking CI setups that treat console
usage as warnings or errors; instead, reference `ref` without logging so the
parameter is "used" without side effects. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Noisy test logs from EchartsTimeseries tests.
- ⚠️ CI warnings/errors when console usage is prohibited.
- ⚠️ Developer workflow: harder test log inspection.
```
</details>
```suggestion
void ref;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest test suite including the file at
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx.
2. The test mock for the Echart component (forwardRef implementation) at
that file
triggers on render (file lines ~45-51). When EchartsTimeseries is rendered
in tests (e.g.,
render(<EchartsTimeseries {...defaultProps} />)), the mocked component is
invoked and
executes the line at 48 (console.log(ref);).
3. Observe console output for every test that mounts EchartsTimeseries: the
ref value is
printed to stdout/stderr during test runs.
4. In CI environments or linting setups that treat console usage as
warnings/errors, this
can cause noisy logs or test failures. Replacing the logging with a no-op
usage (void
ref;) removes the side effect while keeping the variable referenced to
satisfy linters.
```
</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/src/Timeseries/EchartsTimeseries.test.tsx
**Line:** 48:48
**Comment:**
*Possible Bug: The `console.log(ref);` call inside the mocked `Echart`
component is a leftover debug side effect that will run on every test render,
cluttering test output and potentially breaking CI setups that treat console
usage as warnings or errors; instead, reference `ref` without logging so the
parameter is "used" without side effects.
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>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx:
##########
@@ -228,7 +228,7 @@ test('observes extra control height changes when
ResizeObserver is available', a
};
unobserve(_target: Element): void {
- void _target;
+ console.log(_target);
Review Comment:
**Suggestion:** The `console.log(_target);` in the
`MockResizeObserver.unobserve` implementation is an unnecessary debug side
effect that will execute whenever `unobserve` is called, polluting test logs
and potentially causing failures in environments that disallow console output;
you can instead reference `_target` without logging to satisfy unused-parameter
rules. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Noisy test logs from ResizeObserver mock usage.
- ⚠️ CI warnings/errors when console usage is prohibited.
- ⚠️ Harder-to-read test output for EchartsTimeseries tests.
```
</details>
```suggestion
void _target;
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Run the Jest tests that include
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/EchartsTimeseries.test.tsx.
2. The MockResizeObserver class defined in the test (lines ~228-234) is used
when the test
sets global ResizeObserver to MockResizeObserver and mounts EchartsTimeseries
(render(...)).
3. When the code path calls the unobserve method on the mock instance, the
implementation
at line 231 executes console.log(_target); and writes the element to the
test output.
4. This produces console output on every unobserve call (during
unmount/disconnect flows),
creating noisy logs and possibly triggering CI rules that fail on console
usage. Changing
to void _target; removes the side effect while keeping the parameter
referenced for
linters.
```
</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/src/Timeseries/EchartsTimeseries.test.tsx
**Line:** 231:231
**Comment:**
*Possible Bug: The `console.log(_target);` in the
`MockResizeObserver.unobserve` implementation is an unnecessary debug side
effect that will execute whenever `unobserve` is called, polluting test logs
and potentially causing failures in environments that disallow console output;
you can instead reference `_target` without logging to satisfy unused-parameter
rules.
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]
