You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "AndreMeyerr (via GitHub)" <gi...@apache.org> on 2026/06/09 02:18:47 UTC
[PR] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
AndreMeyerr opened a new pull request, #40878:
URL: https://github.com/apache/superset/pull/40878
## Summary
Adds dedicated unit tests for `get_current_user` in `superset/tasks/utils.py`.
This function had no prior unit test coverage in the repository.
## Motivation
`get_current_user` is used to resolve the executor of scheduled async tasks
(e.g. thumbnail generation via `get_executor`). Despite its role in task
execution, it had no dedicated unit tests — only integration-level references
where it was mocked as a dependency of other tests.
## Changes
- Added `tests/unit_tests/tasks/get_current_user.py` with 7 test cases
- Techniques: equivalence partitioning (black-box) + branch/MC/DC (white-box)
- Isolation: module loaded via `importlib` + `patch.object` on `g` to avoid
requiring a full Superset application context
## Test cases
| ID | Condition | Expected |
|---|---|---|
| CT01 | `g.user` attribute absent | `None` |
| CT02 | `g.user = None` (falsy) | `None` |
| CT03 | anonymous user (`is_anonymous=True`) | `None` |
| CT04 | authenticated user | `username` |
| CT05 | `g.user = 0` (non-None falsy) | `None` |
| CT06 | `username = ""` (boundary) | `""` |
| CT07 | `username = None` (boundary) | `None` |
## Execution
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "AndreMeyerr (via GitHub)" <gi...@apache.org>.
AndreMeyerr commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4680633862
Hi, could a maintainer please approve the CI workflows to run for this PR?
I've addressed the reviewer feedback from @aminghadersohi — moved the tests
to test_utils.py, rewrote everything in English, added type hints, and removed
the importlib/stub approach. Thanks!
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4738584128
<!-- translation-regression-bot -->
## ⚠️ Translation Regression Detected
A source change in this PR renamed or reworded strings, invalidating existing translations (they are now `#, fuzzy`) in `de`, `fi`, `th`. Please resolve the affected `.po` files before merging.
_Note: intentionally **deleting** a translatable string is not a regression and is not flagged here — only translations invalidated by a renamed/reworded source string are._
| Language | Fuzzy before | Fuzzy after | New |
|----------|-------------:|------------:|----:|
| `de` | 0 | 997 | +997 |
| `fi` | 4817 | 4823 | +6 |
| `th` | 4817 | 4823 | +6 |
### How to fix
**1. Install dependencies** (if not already set up):
```bash
pip install -r superset/translations/requirements.txt
sudo apt-get install gettext # or: brew install gettext
```
**2. Re-extract strings and sync `.po` files:**
```bash
./scripts/translations/babel_update.sh
```
This rewrites `superset/translations/messages.pot` from the current source files and merges the changes into every `.po` file. Strings whose `msgid` changed will be marked `#, fuzzy`.
**3. Resolve the fuzzy entries** in the affected language files (`de`, `fi`, `th`):
```bash
grep -n '#, fuzzy' superset/translations/<lang>/LC_MESSAGES/messages.po
```
For each fuzzy entry, either rewrite the `msgstr` to match the new string and remove the `#, fuzzy` line, or clear the `msgstr` to `""` if you cannot provide a translation.
**4. Commit your changes to the `.po` files.**
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40878:
URL: https://github.com/apache/superset/pull/40878#discussion_r3377465595
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
Review Comment:
**🟠 Architect Review — HIGH**
The test module mutates global `sys.modules` at import time with broad stubs (e.g. `superset.utils`, `superset.utils.hashing`, `superset.tasks.types`, `superset_core`) via `_stub(...)` and never restores them, so these fake modules replace the real packages for all later tests in the same process. Given pytest's deterministic collection order (`tests/unit_tests/tasks/test_get_current_user.py` is imported before `tests/unit_tests/utils/test_hashing.py` and other tests that import these modules), this injects cross-test contamination and causes order-dependent failures when other tests expect the real implementations.
**Suggestion:** Avoid permanent `sys.modules` mutation by either importing `superset.tasks.utils` directly as in `tests/unit_tests/tasks/test_utils.py:1-14` (so normal imports resolve real dependencies), or, if module-level stubbing is required, follow the existing pattern used in `tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py:15-55` and `tests/unit_tests/mcp_service/sql_lab/tool/test_save_sql_query.py:8-50` to save original `sys.modules` entries and restore them after each import.
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=56edf3d0135f479384d3fdc1716f6c8e&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=56edf3d0135f479384d3fdc1716f6c8e&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 an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** tests/unit_tests/tasks/test_get_current_user.py
**Line:** 42:76
**Comment:**
*HIGH: The test module mutates global `sys.modules` at import time with broad stubs (e.g. `superset.utils`, `superset.utils.hashing`, `superset.tasks.types`, `superset_core`) via `_stub(...)` and never restores them, so these fake modules replace the real packages for all later tests in the same process. Given pytest's deterministic collection order (`tests/unit_tests/tasks/test_get_current_user.py` is imported before `tests/unit_tests/utils/test_hashing.py` and other tests that import these modules), this injects cross-test contamination and causes order-dependent failures when other tests expect the real implementations.
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.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
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>
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4681872026
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #40878:
URL: https://github.com/apache/superset/pull/40878
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4655426574
<!-- Bito Reply -->
The architectural review finding is correct. The current implementation of `_stub` permanently modifies `sys.modules`, which causes cross-test contamination. To resolve this, you should save the original `sys.modules` state and restore it after the module is loaded, or use a context manager to handle the temporary stubs.
Below is a minimal fix using a context manager to ensure `sys.modules` is restored after the module is loaded:
```python
import sys
from contextlib import contextmanager
@contextmanager
def temporary_stubs(stubs):
original = {name: sys.modules.get(name) for name in stubs}
try:
for name, mod in stubs.items():
sys.modules[name] = mod
yield
finally:
for name, original_mod in original.items():
if original_mod is None:
sys.modules.pop(name, None)
else:
sys.modules[name] = original_mod
# Usage:
# stubs = {"celery": _stub("celery"), ...}
# with temporary_stubs(stubs):
# _spec.loader.exec_module(_mod)
```
Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?
**tests/unit_tests/tasks/test_get_current_user.py**
```
@contextmanager
def temporary_stubs(stubs):
original = {name: sys.modules.get(name) for name in stubs}
try:
for name, mod in stubs.items():
sys.modules[name] = mod
yield
finally:
for name, original_mod in original.items():
if original_mod is None:
sys.modules.pop(name, None)
else:
sys.modules[name] = original_mod
```
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #40878:
URL: https://github.com/apache/superset/pull/40878#discussion_r3382593776
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
Review Comment:
The module docstring appears to have mojibake/encoding issues (eg `unitários`, `Técnicas`, `substituição`) and is not consistent with the rest of the unit-test suite which is written in English. This makes the file harder to read/search and suggests it may have been saved/decoded with the wrong encoding.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
Review Comment:
This test module mutates `sys.modules` at import time (via `sys.modules.setdefault(...)`) to inject stub modules. If any of these modules (e.g. `celery`, `superset.utils`, etc.) have not been imported yet, the stubs will become the canonical modules for the rest of the test session, which can break unrelated tests depending on import order.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+get_current_user = _mod.get_current_user
+
+
+@pytest.fixture
+def app():
+ application = Flask(__name__)
+ application.config["TESTING"] = True
+ return application
+
+
+class TestGetCurrentUser:
+ def test_retorna_none_quando_g_nao_possui_user(self, app):
+ """CT01 | CI1 | D1=F (hasattr=False, curto-circuito)."""
+ mock_g = MagicMock(spec=[]) # spec=[] → hasattr(g, "user") == False
+ with app.app_context():
Review Comment:
Test names and inline comments use Portuguese identifiers/text (eg `test_retorna_*`, `usuario`, `curto-circuito`), which is inconsistent with nearby unit tests in this directory (eg `tests/unit_tests/tasks/test_utils.py`) and makes the suite harder to maintain for the broader contributor base. Consider renaming tests/variables/comments to English for consistency.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+get_current_user = _mod.get_current_user
+
+
+@pytest.fixture
+def app():
+ application = Flask(__name__)
+ application.config["TESTING"] = True
+ return application
+
+
+class TestGetCurrentUser:
+ def test_retorna_none_quando_g_nao_possui_user(self, app):
+ """CT01 | CI1 | D1=F (hasattr=False, curto-circuito)."""
+ mock_g = MagicMock(spec=[]) # spec=[] → hasattr(g, "user") == False
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_g_user_e_none(self, app):
+ """CT02 | CI2 | D1=F (hasattr=True, g.user falsy)."""
+ mock_g = MagicMock()
+ mock_g.user = None
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_usuario_anonimo(self, app):
+ """CT03 | CI3 | D1=V; D2=F (is_anonymous=True)."""
+ usuario = MagicMock()
+ usuario.is_anonymous = True
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_username_quando_usuario_autenticado(self, app):
+ """CT04 | CV1 | D1=V; D2=V."""
+ usuario = MagicMock()
+ usuario.is_anonymous = False
+ usuario.username = "andre"
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() == "andre"
+
+ def test_retorna_none_quando_g_user_e_falsy_nao_none(self, app):
+ """CT05 | CI2 ampliada | D1=F (g.user = 0, falsy mas não None).
+
+ Verifica que qualquer valor falsy em g.user (não apenas None) encerra
+ o fluxo antes de avaliar is_anonymous, retornando None.
+ """
Review Comment:
This docstring contains mojibake/encoding artifacts (eg `não`) which should be corrected (likely by saving the file as UTF-8) and/or rewritten in plain ASCII/English for readability.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+get_current_user = _mod.get_current_user
+
+
+@pytest.fixture
+def app():
+ application = Flask(__name__)
+ application.config["TESTING"] = True
+ return application
+
+
+class TestGetCurrentUser:
+ def test_retorna_none_quando_g_nao_possui_user(self, app):
+ """CT01 | CI1 | D1=F (hasattr=False, curto-circuito)."""
+ mock_g = MagicMock(spec=[]) # spec=[] → hasattr(g, "user") == False
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_g_user_e_none(self, app):
+ """CT02 | CI2 | D1=F (hasattr=True, g.user falsy)."""
+ mock_g = MagicMock()
+ mock_g.user = None
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_usuario_anonimo(self, app):
+ """CT03 | CI3 | D1=V; D2=F (is_anonymous=True)."""
+ usuario = MagicMock()
+ usuario.is_anonymous = True
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_username_quando_usuario_autenticado(self, app):
+ """CT04 | CV1 | D1=V; D2=V."""
+ usuario = MagicMock()
+ usuario.is_anonymous = False
+ usuario.username = "andre"
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() == "andre"
+
+ def test_retorna_none_quando_g_user_e_falsy_nao_none(self, app):
+ """CT05 | CI2 ampliada | D1=F (g.user = 0, falsy mas não None).
+
+ Verifica que qualquer valor falsy em g.user (não apenas None) encerra
+ o fluxo antes de avaliar is_anonymous, retornando None.
+ """
+ mock_g = MagicMock()
+ mock_g.user = 0
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_string_vazia_quando_username_e_vazio(self, app):
+ """CT06 | CV1 borda | D1=V; D2=V; username = "".
+
+ Comportamento não especificado formalmente — a função retorna string
+ vazia em vez de None quando user.username == "".
+ """
+ usuario = MagicMock()
+ usuario.is_anonymous = False
+ usuario.username = ""
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() == ""
+
+ def test_retorna_none_quando_username_e_none(self, app):
+ """CT07 | CV1 borda | D1=V; D2=V; username = None.
+
+ username=None retorna None via user.username, não pelo fluxo principal
+ (ou seja, a guarda `not user.is_anonymous` é satisfeita, mas o valor
+ retornado é None porque user.username é None).
Review Comment:
This docstring contains mojibake/encoding artifacts (eg `não`, `é`). Consider rewriting it in English and ensuring the file is saved as UTF-8 to avoid unreadable text in the repo.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+get_current_user = _mod.get_current_user
+
+
+@pytest.fixture
+def app():
+ application = Flask(__name__)
+ application.config["TESTING"] = True
+ return application
+
+
+class TestGetCurrentUser:
+ def test_retorna_none_quando_g_nao_possui_user(self, app):
+ """CT01 | CI1 | D1=F (hasattr=False, curto-circuito)."""
+ mock_g = MagicMock(spec=[]) # spec=[] → hasattr(g, "user") == False
Review Comment:
This inline comment contains an encoding artifact (`→`). Use plain ASCII (`->`) to avoid mojibake in the repo and keep comments searchable.
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
+
+get_current_user = _mod.get_current_user
+
+
+@pytest.fixture
+def app():
+ application = Flask(__name__)
+ application.config["TESTING"] = True
+ return application
+
+
+class TestGetCurrentUser:
+ def test_retorna_none_quando_g_nao_possui_user(self, app):
+ """CT01 | CI1 | D1=F (hasattr=False, curto-circuito)."""
+ mock_g = MagicMock(spec=[]) # spec=[] → hasattr(g, "user") == False
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_g_user_e_none(self, app):
+ """CT02 | CI2 | D1=F (hasattr=True, g.user falsy)."""
+ mock_g = MagicMock()
+ mock_g.user = None
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_none_quando_usuario_anonimo(self, app):
+ """CT03 | CI3 | D1=V; D2=F (is_anonymous=True)."""
+ usuario = MagicMock()
+ usuario.is_anonymous = True
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_username_quando_usuario_autenticado(self, app):
+ """CT04 | CV1 | D1=V; D2=V."""
+ usuario = MagicMock()
+ usuario.is_anonymous = False
+ usuario.username = "andre"
+ mock_g = MagicMock()
+ mock_g.user = usuario
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() == "andre"
+
+ def test_retorna_none_quando_g_user_e_falsy_nao_none(self, app):
+ """CT05 | CI2 ampliada | D1=F (g.user = 0, falsy mas não None).
+
+ Verifica que qualquer valor falsy em g.user (não apenas None) encerra
+ o fluxo antes de avaliar is_anonymous, retornando None.
+ """
+ mock_g = MagicMock()
+ mock_g.user = 0
+ with app.app_context():
+ with patch.object(_mod, "g", mock_g):
+ assert get_current_user() is None
+
+ def test_retorna_string_vazia_quando_username_e_vazio(self, app):
+ """CT06 | CV1 borda | D1=V; D2=V; username = "".
+
+ Comportamento não especificado formalmente — a função retorna string
+ vazia em vez de None quando user.username == "".
+ """
Review Comment:
This docstring includes mojibake/encoding artifacts (eg `não`, `—`). Rewriting it in English (and ensuring the file is UTF-8) will keep the test suite consistent and readable.
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4680593614
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 5b600525595cc1faaa7192d09473cce2872f1e96 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a2aa0bf99584300084b848b |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-40878--superset-docs-preview.netlify.app](https://deploy-preview-40878--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcrLDYAgDADQXXqWSBQNuoFjFKmfRIGUcjDG3eX2Du-FxPFOAjNAA44xrEf1RihtCacooSxZ7SRqLcwURJVMXK-ndMVn8XWP2CFqt03TYE2vtbbGWWMdfD8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-40878--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4655404952
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "AndreMeyerr (via GitHub)" <gi...@apache.org>.
AndreMeyerr commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4708301908
@aminghadersohi Fixed the redundant decorator in test_returns_none_when_g_has_no_user_attribute — dropped the outer @patch and kept only the context manager. Thanks for the thorough review!
--
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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4709963485
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4754020766
**Bito Automatic Review Skipped – PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save 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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #40878:
URL: https://github.com/apache/superset/pull/40878#discussion_r3377481432
##########
tests/unit_tests/tasks/test_get_current_user.py:
##########
@@ -0,0 +1,165 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Testes unitários para get_current_user (superset/tasks/utils.py).
+
+Técnicas aplicadas:
+ - Caixa-preta : particionamento de equivalência (CV1, CI1-CI3)
+ - Caixa-branca: cobertura de branch + MC/DC (D1, D2)
+ - Isolamento : substituição de g via Flask app_context + patch direto
+"""
+
+import importlib.util
+import pathlib
+import sys
+import types
+from unittest.mock import MagicMock, patch
+
+import pytest
+from flask import Flask
+
+
+def _stub(name, **attrs):
+ mod = types.ModuleType(name)
+ mod.__dict__.update(attrs)
+ sys.modules.setdefault(name, mod)
+ return mod
+
+
+_stub("celery")
+_stub("celery.utils")
+_stub("celery.utils.log", get_task_logger=lambda n: MagicMock())
+_stub("superset_core")
+_stub("superset_core.tasks")
+_stub("superset_core.tasks.types", TaskProperties=dict, TaskScope=MagicMock())
+_stub(
+ "superset.tasks.exceptions",
+ ExecutorNotFoundError=Exception,
+ InvalidExecutorError=Exception,
+)
+_stub(
+ "superset.tasks.types",
+ ChosenExecutor=MagicMock(),
+ Executor=MagicMock(),
+ ExecutorType=MagicMock(),
+ FixedExecutor=MagicMock(),
+)
+_stub("superset.utils")
+_stub(
+ "superset.utils.json",
+ loads=MagicMock(),
+ dumps=MagicMock(),
+ JSONDecodeError=ValueError,
+)
+_stub("superset.utils.hashing", hash_from_str=MagicMock(return_value="abc" * 30))
+_stub("superset.utils.urls", get_url_path=MagicMock())
+
+_path = pathlib.Path(__file__).parents[3] / "superset" / "tasks" / "utils.py"
+_spec = importlib.util.spec_from_file_location("superset.tasks.utils", _path)
+assert _spec is not None
+assert _spec.loader is not None
+_mod = importlib.util.module_from_spec(_spec)
+_spec.loader.exec_module(_mod)
Review Comment:
**Suggestion:** This test bootstraps `superset.tasks.utils` via `exec_module` after stubbing only deep submodules, but it does not stub the parent `superset` package chain. During `from superset.tasks.exceptions import ...`, Python can still import the real `superset` package (triggering heavy app/bootstrap imports from `superset/__init__.py`), which makes this unit test fail in minimal environments and introduces import-order fragility. Stub the parent packages (`superset`, `superset.tasks`) or load the target module under an isolated module name with a fully patched module graph. [import error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- CRIT: Unit tests import full Superset package unnecessarily.
- WARN: Test requires Celery and SQLAlchemy just to import.
- WARN: Harder to run test in minimal environments.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Import `tests/unit_tests/tasks/test_get_current_user.py`, which executes the `_stub`
and import bootstrapping logic at lines 42–75 (notably `_stub("superset.tasks.exceptions",
...)` at lines 48–52 and `_spec.loader.exec_module(_mod)` at line 75).
2. During `_spec.loader.exec_module(_mod)`, the real module `superset/tasks/utils.py` is
executed under the name `"superset.tasks.utils"` (see `superset/tasks/utils.py` lines
27–40 for its top-level imports).
3. While executing `superset/tasks/utils.py`, the `from superset.tasks.exceptions import
ExecutorNotFoundError, InvalidExecutorError` statement at line 31 triggers Python's import
machinery to load the real `superset` package hierarchy, executing `superset/__init__.py`
lines 17–31, which in turn imports `superset.app` and `superset.extensions`.
4. Importing `superset.extensions` at `superset/extensions/__init__.py` lines 17–61 and
139–160 runs significant side-effectful initialization (Celery app, SQLAlchemy `db`, cache
manager, feature flags, etc.), meaning every run of this unit test implicitly bootstraps
the full Superset package instead of remaining isolated to `superset.tasks.utils`.
```
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=28c6940ada764a4fabe33ed807717990&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=28c6940ada764a4fabe33ed807717990&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/tasks/test_get_current_user.py
**Line:** 45:75
**Comment:**
*Import Error: This test bootstraps `superset.tasks.utils` via `exec_module` after stubbing only deep submodules, but it does not stub the parent `superset` package chain. During `from superset.tasks.exceptions import ...`, Python can still import the real `superset` package (triggering heavy app/bootstrap imports from `superset/__init__.py`), which makes this unit test fail in minimal environments and introduces import-order fragility. Stub the parent packages (`superset`, `superset.tasks`) or load the target module under an isolated module name with a fully patched module graph.
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%2F40878&comment_hash=1d125d95a5d978d0db255563128e14725e763abebfca9a280334e05988c9b764&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40878&comment_hash=1d125d95a5d978d0db255563128e14725e763abebfca9a280334e05988c9b764&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: 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] test: add unit tests for get_current_user (superset/tasks/utils.py) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #40878:
URL: https://github.com/apache/superset/pull/40878#issuecomment-4662380032
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/40878?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 68.30%. Comparing base ([`1188cfe`](https://app.codecov.io/gh/apache/superset/commit/1188cfef1d4ebd7d7c6637b3bf551cc7756f1e56?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`5ae0873`](https://app.codecov.io/gh/apache/superset/commit/5ae087349fdb00453d357ad6e304bfa2d266ec79?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 32 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #40878 +/- ##
=======================================
Coverage 68.30% 68.30%
=======================================
Files 1865 1865
Lines 77997 77997
Branches 25032 25032
=======================================
Hits 53274 53274
Misses 24723 24723
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/40878/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [python](https://app.codecov.io/gh/apache/superset/pull/40878/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [unit](https://app.codecov.io/gh/apache/superset/pull/40878/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
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/40878?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