You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/06/23 03:50:57 UTC
[PR] fix(tests): stabilize update_chart MCP test failing on previous-Python CI leg [superset]
rusackas opened a new pull request, #41310:
URL: https://github.com/apache/superset/pull/41310
### SUMMARY
The `unit-tests (previous)` (and the aggregate `unit-tests-required`) CI leg has been **red on roughly half of recent master commits** — not flakiness, but a single deterministic failure:
```
tests/unit_tests/mcp_service/chart/tool/test_update_chart.py::TestUpdateChartDatasetIdIntegration::test_dataset_id_passed_to_update_command
AttributeError: <function update_chart ...> does not have the attribute '_validate_update_against_dataset'
```
**Root cause (a logical merge conflict):** `superset/mcp_service/chart/tool/__init__.py` re-exports the `update_chart` *function* (`from .update_chart import update_chart`), which shadows the `update_chart` **submodule** attribute on the `tool` package. The failing test used a string-based decorator:
```python
@patch("superset.mcp_service.chart.tool.update_chart._validate_update_against_dataset", ...)
```
`unittest.mock` resolves that dotted path via `getattr(tool, "update_chart")`, which returns the **function** rather than the module, so the attribute lookup raises `AttributeError`. Older `unittest.mock` returns the shadowing attribute while newer versions prefer the submodule — which is exactly why only the **previous-Python** matrix leg fails while the current-Python leg passes, letting it slip past per-PR CI and land on master repeatedly.
The other three tests in this same file already anticipate the shadowing and patch via the `importlib`-resolved module object:
```python
update_chart_module = importlib.import_module("superset.mcp_service.chart.tool.update_chart")
...
@patch.object(update_chart_module, "_validate_update_against_dataset", ...)
```
This PR makes the one remaining test consistent with them. One-line-scope change; no production code touched.
### TESTING INSTRUCTIONS
Verified locally on **Python 3.10** (the previous-version leg):
```bash
python -m pytest "tests/unit_tests/mcp_service/chart/tool/test_update_chart.py::TestUpdateChartDatasetIdIntegration::test_dataset_id_passed_to_update_command"
```
- Before: fails with the `AttributeError` above.
- After: passes.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tests): stabilize update_chart MCP test failing on previous-Python CI leg [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41310:
URL: https://github.com/apache/superset/pull/41310#issuecomment-4775480270
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.59%. Comparing base ([`5916ec4`](https://app.codecov.io/gh/apache/superset/commit/5916ec4876a1d3594dbbe139b62a2fc5309a4c5a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`11f820b`](https://app.codecov.io/gh/apache/superset/commit/11f820b9db09a635d4ec1eb696a1df18806ebf5c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41310 +/- ##
=======================================
Coverage 63.59% 63.59%
=======================================
Files 2653 2653
Lines 144952 144952
Branches 33433 33433
=======================================
Hits 92180 92180
Misses 51082 51082
Partials 1690 1690
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41310/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [hive](https://app.codecov.io/gh/apache/superset/pull/41310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.27% <ø> (ø)` | |
| [presto](https://app.codecov.io/gh/apache/superset/pull/41310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.86% <ø> (ø)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/41310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.88% <ø> (ø)` | |
| [sqlite](https://app.codecov.io/gh/apache/superset/pull/41310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.72% <ø> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tests): stabilize update_chart MCP test failing on previous-Python CI leg [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41310:
URL: https://github.com/apache/superset/pull/41310#issuecomment-4787628747
@aminghadersohi does that mean you'd approve it? 🤞
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tests): stabilize update_chart MCP test failing on previous-Python CI leg [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41310:
URL: https://github.com/apache/superset/pull/41310#issuecomment-4775432664
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(tests): stabilize update_chart MCP test failing on previous-Python CI leg [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41310:
URL: https://github.com/apache/superset/pull/41310
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org