codeant-ai-for-open-source[bot] commented on code in PR #40647:
URL: https://github.com/apache/superset/pull/40647#discussion_r3401936077
##########
tests/integration_tests/email_tests.py:
##########
@@ -208,6 +208,7 @@ def test_send_mime(self, mock_smtp, mock_smtp_ssl):
@mock.patch("smtplib.SMTP")
def test_send_mime_ssl(self, mock_smtp, mock_smtp_ssl):
current_app.config["SMTP_SSL"] = True
+ current_app.config["SMTP_SSL_SERVER_AUTH"] = False
Review Comment:
**Suggestion:** This test mutates global `current_app.config` and never
restores `SMTP_SSL_SERVER_AUTH`, which makes later tests order-dependent when
they rely on the default value. Save the original value and restore it in a
`finally` block (or use a fixture/`patch.dict`) so this test remains isolated
and cannot leak state to other tests. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Later email tests may see wrong TLS verification default.
- ⚠️ Test suite may become order-dependent and intermittently flaky.
- ⚠️ Future tests using defaults risk unexpected SMTP behavior.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Note that a single global Flask app instance is created for all
integration tests in
`tests/integration_tests/test_app.py:28-32` via `app = create_app(...)`, and
`SupersetTestCase.create_app()` returns this same `app` for each test case in
`tests/integration_tests/base_tests.py:111-127`, so `current_app.config`
mutations persist
across tests in the same process.
2. In `tests/integration_tests/email_tests.py:39-42`,
`TestEmailSmtp.setUp()` only resets
`current_app.config["SMTP_SSL"] = False` and does not touch
`SMTP_SSL_SERVER_AUTH`,
meaning whatever value this key had from a previous test is carried into the
next one.
3. The test `TestEmailSmtp.test_send_mime_ssl` at
`tests/integration_tests/email_tests.py:209-223` explicitly sets
`current_app.config["SMTP_SSL_SERVER_AUTH"] = False` on line 211 and never
restores it to
its prior value, unlike other tests in the same file that save and restore
mutated config
(e.g. `EMAIL_HEADER_MUTATOR` at lines 69-92 and `SMTP_USER`/`SMTP_PASSWORD`
at lines 7-23
in the later `test_send_mime_noauth`).
4. Since the default for `SMTP_SSL_SERVER_AUTH` is defined as `True` in
`superset/config.py:1696-1709`, any later test in the same process that
expects this
default (and does not override the key in its own setup) will instead see
the leaked
`False` value from `test_send_mime_ssl`, making the suite order-dependent
and causing
future tests that rely on the default TLS verification behavior in
`superset/utils/core.py:930-32` to behave differently based solely on
execution order.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=aacc82d9a75b4feabfce0786adeae2f1&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=aacc82d9a75b4feabfce0786adeae2f1&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/integration_tests/email_tests.py
**Line:** 211:211
**Comment:**
*Logic Error: This test mutates global `current_app.config` and never
restores `SMTP_SSL_SERVER_AUTH`, which makes later tests order-dependent when
they rely on the default value. Save the original value and restore it in a
`finally` block (or use a fixture/`patch.dict`) so this test remains isolated
and cannot leak state to other tests.
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%2F40647&comment_hash=e52c8813e169c81ad73c784b0fbecf5926d972cb3ae92d210d103d6b7cd00b1c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40647&comment_hash=e52c8813e169c81ad73c784b0fbecf5926d972cb3ae92d210d103d6b7cd00b1c&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]
