codeant-ai-for-open-source[bot] commented on code in PR #37516:
URL: https://github.com/apache/superset/pull/37516#discussion_r3557432103
##########
tests/unit_tests/common/test_query_context_processor.py:
##########
@@ -1707,6 +1707,10 @@ def __init__(self):
assert mock_cache.is_loaded is False, (
"Cache should be inv when no applied_filter_columns and query has
filters"
)
+ assert mock_cache.is_cached is None
+ assert mock_cache.cache_dttm is None
+ assert mock_cache.cache_value is None
Review Comment:
**Suggestion:** The new `cache_value` assertion is ineffective because the
fixture initializes `cache_value` to `None` before calling `get_df_payload`, so
this check passes even if cache invalidation stops clearing that field.
Initialize `cache_value` to a non-`None` sentinel value in `MockCache` so this
assertion actually verifies the invalidation behavior. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Test cannot detect regressions in cache_value invalidation.
- ⚠️ Cache invalidation behavior less protected against future changes.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `tests/unit_tests/common/test_query_context_processor.py` and locate
`test_get_df_payload_invalidates_cache_missing_applied_filter_columns`
starting around
line 1643 (verified via Read tool).
2. Inside this test, observe the nested `MockCache` class definition at
lines 29–51, where
`__init__` sets `self.cache_value = None` (line 43 in the current file
content).
3. Continue down in the same test to the verification block after
`processor.get_df_payload(...)` is called (lines 8–15 of the second
snippet): the new
assertion `assert mock_cache.cache_value is None` appears at diff line 1712.
4. Note that no code path in this test ever assigns a non-`None` value to
`mock_cache.cache_value`; it is initialized as `None` and never changed, so
the assertion
`assert mock_cache.cache_value is None` will always pass, even if future
changes stop
clearing `cache_value` during cache invalidation, making the assertion
ineffective for
detecting regressions.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9ff40931a7bc497b95953fbc2ad39592&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9ff40931a7bc497b95953fbc2ad39592&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** tests/unit_tests/common/test_query_context_processor.py
**Line:** 1712:1712
**Comment:**
*Incomplete Implementation: The new `cache_value` assertion is
ineffective because the fixture initializes `cache_value` to `None` before
calling `get_df_payload`, so this check passes even if cache invalidation stops
clearing that field. Initialize `cache_value` to a non-`None` sentinel value in
`MockCache` so this assertion actually verifies the invalidation behavior.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=1f0150552e0dec3b03d5c7689afce8e42d12e244603e6cc6b9c10642dae257e8&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37516&comment_hash=1f0150552e0dec3b03d5c7689afce8e42d12e244603e6cc6b9c10642dae257e8&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]
