aminghadersohi commented on PR #44386: URL: https://github.com/apache/superset/pull/44386#issuecomment-5756204790
@rebenitez1802 Following up on both review bodies ([21:19 review](https://github.com/apache/superset/pull/44386#pullrequestreview-5261767284), [21:33 review](https://github.com/apache/superset/pull/44386#pullrequestreview-5261801845)): the swallowed-`StopIteration` concern is valid test-quality feedback, not cosmetic. The non-exhausting over-budget mocks, already-fits preservation case, opaque `get_chart_sql`/`execute_sql` coverage, and success-assumption comment were added in 5763594911a0f7861f614d28fdf2d95905c6d13a. Closing the verification gap in 83f1ebcbc43b6de1d41c2f3dd6c873065a150780: added parameterized shrink tests using the real estimator for oversized chart identity, structured errors, and URLs. These verify actual over-budget inputs become bounded while preserving identity, preview state, and failure status. Deliberate-break evidence (all temporary mutations restored): - Disabled `_shrink_minimal_response`: **5 failed, 1 passed**, including the existing oversized-field and structured-error tests plus all three real-estimator cases. - Removed the `_fits` early return: **1 failed**, the already-fits preservation test. - Narrowed `_fits`'s exception handler to `(TypeError, ValueError)`: **18 passed**, confirming the minimal-response tests do not depend on swallowing `StopIteration`. - Restored production code: **251 affected tests passed**; staged and branch-scoped `pre-commit run` gates and explicit `pre-commit run mypy` passed. Both review bodies are addressed; GitHub has no body-level resolve operation or associated unresolved thread, so this linked reply records closure. I have not re-requested review or dismissed either approval. Acceptance criteria remain disclosed: **AC 3 is PARTIAL** (oversized-response truncation is handled, but `update_chart`'s default payload is not slimmed); **AC 6 remains post-deploy Datadog verification**. Neither is being marked complete. -- 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]
