ferruzzi commented on code in PR #63994:
URL: https://github.com/apache/airflow/pull/63994#discussion_r3029672066
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_variables.py:
##########
@@ -521,6 +521,23 @@ def test_patch_should_respond_404(self, test_client):
body = response.json()
assert f"The Variable with key: `{TEST_VARIABLE_KEY}` was not found"
== body["detail"]
+ @conf_vars({("core", "multi_team"): "False"})
+ def test_patch_rejects_team_name_when_multi_team_disabled(self,
test_client, testing_team):
Review Comment:
I've poked at your failing SQL test and I think the issue is the
`testing_team` fixture. It's holding an open DB connection, keeping it locked
while your tests run, and your test is trying to access the locked row.
Fortunately, your tests don't actually need a valid team to exist, so you
should be able to drop that fixture and just use a made-up team name. In
theory, your code should never get tot he point where it's validating the team
name, right? `If multi-team is disabled and a team name is provided then....`
doesn't actually require any db access.
Whichever route you use for fixing this, make sure you apply it to this test
and to test_patch_rejects_team_name_when_multi_team_disabled as well, and maybe
have a look to see if other tests are repeating that pattern... if we don't
need DB access, then we shouldn't be using that fixture. I'd even argue that
using that fixture when it isn't needed may be hiding some other underlying
"short circuit" cutouts like this.
--
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]
