mirror of
https://github.com/zulip/zulip.git
synced 2025-11-03 21:43:21 +00:00
transaction_tests: Remove testing URL.
Rewrite the test so that we don't have a dedicated URL for testing. dev_update_subgroups is called directly from the tests without using the test client.
This commit is contained in:
Zixuan James Li
committed by
Tim Abbott
Tim Abbott
parent
81bd63cb46
commit
1e1f98edb2
@@ -1,16 +1,63 @@
|
||||
import threading
|
||||
from typing import TYPE_CHECKING, List, Optional
|
||||
from typing import Any, List, Optional
|
||||
from unittest import mock
|
||||
|
||||
import orjson
|
||||
from django.db import connections, transaction
|
||||
from django.db import OperationalError, connections, transaction
|
||||
from django.http import HttpRequest
|
||||
|
||||
from zerver.actions.user_groups import add_subgroups_to_user_group, check_add_user_group
|
||||
from zerver.lib.exceptions import JsonableError
|
||||
from zerver.lib.test_classes import ZulipTransactionTestCase
|
||||
from zerver.models import Realm, UserGroup, get_realm
|
||||
from zerver.views.development import user_groups as user_group_view
|
||||
from zerver.lib.test_helpers import HostRequestMock
|
||||
from zerver.lib.user_groups import access_user_group_by_id
|
||||
from zerver.models import Realm, UserGroup, UserProfile, get_realm
|
||||
from zerver.views.user_groups import update_subgroups_of_user_group
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from django.test.client import _MonkeyPatchedWSGIResponse as TestHttpResponse
|
||||
BARRIER: Optional[threading.Barrier] = None
|
||||
|
||||
|
||||
def dev_update_subgroups(
|
||||
request: HttpRequest,
|
||||
user_profile: UserProfile,
|
||||
user_group_id: int,
|
||||
) -> Optional[str]:
|
||||
# The test is expected to set up the barrier before accessing this endpoint.
|
||||
assert BARRIER is not None
|
||||
try:
|
||||
with transaction.atomic(), mock.patch(
|
||||
"zerver.lib.user_groups.access_user_group_by_id"
|
||||
) as m:
|
||||
|
||||
def wait_after_recursive_query(*args: Any, **kwargs: Any) -> UserGroup:
|
||||
# When updating the subgroups, we access the supergroup group
|
||||
# only after finishing the recursive query.
|
||||
BARRIER.wait()
|
||||
return access_user_group_by_id(*args, **kwargs)
|
||||
|
||||
m.side_effect = wait_after_recursive_query
|
||||
|
||||
update_subgroups_of_user_group(request, user_profile, user_group_id=user_group_id)
|
||||
except OperationalError as err:
|
||||
msg = str(err)
|
||||
if "deadlock detected" in msg:
|
||||
return "Deadlock detected"
|
||||
else:
|
||||
assert "could not obtain lock" in msg
|
||||
# This error is possible when nowait is set the True, which only
|
||||
# applies to the recursive query on the subgroups. Because the
|
||||
# recursive query fails, this thread must have not waited on the
|
||||
# barrier yet.
|
||||
BARRIER.wait()
|
||||
return "Busy lock detected"
|
||||
except (
|
||||
threading.BrokenBarrierError
|
||||
): # nocoverage # This is only possible when timeout happens or there is a programming error
|
||||
raise JsonableError(
|
||||
"Broken barrier. The tester should make sure that the exact number of parties have waited on the barrier set by the previous immediate set_sync_after_first_lock call"
|
||||
)
|
||||
|
||||
return None
|
||||
|
||||
|
||||
class UserGroupRaceConditionTestCase(ZulipTransactionTestCase):
|
||||
@@ -46,7 +93,7 @@ class UserGroupRaceConditionTestCase(ZulipTransactionTestCase):
|
||||
def test_lock_subgroups_with_respect_to_supergroup(self) -> None:
|
||||
realm = get_realm("zulip")
|
||||
self.login("iago")
|
||||
test_case = self
|
||||
iago = self.example_user("iago")
|
||||
|
||||
class RacingThread(threading.Thread):
|
||||
def __init__(
|
||||
@@ -55,15 +102,16 @@ class UserGroupRaceConditionTestCase(ZulipTransactionTestCase):
|
||||
supergroup_id: int,
|
||||
) -> None:
|
||||
threading.Thread.__init__(self)
|
||||
self.response: Optional["TestHttpResponse"] = None
|
||||
self.response: Optional[str] = None
|
||||
self.subgroup_ids = subgroup_ids
|
||||
self.supergroup_id = supergroup_id
|
||||
|
||||
def run(self) -> None:
|
||||
try:
|
||||
self.response = test_case.client_post(
|
||||
url=f"/testing/user_groups/{self.supergroup_id}/subgroups",
|
||||
info={"add": orjson.dumps(self.subgroup_ids).decode()},
|
||||
self.response = dev_update_subgroups(
|
||||
HostRequestMock({"add": orjson.dumps(self.subgroup_ids).decode()}),
|
||||
iago,
|
||||
user_group_id=self.supergroup_id,
|
||||
)
|
||||
finally:
|
||||
# Close all thread-local database connections
|
||||
@@ -81,9 +129,8 @@ real subgroup update endpoint by synchronizing them after the acquisition of the
|
||||
first lock in the critical region. Though unlikely, this test might fail as we
|
||||
have no control over the scheduler when the barrier timeouts.
|
||||
""".strip()
|
||||
barrier = threading.Barrier(parties=2, timeout=3)
|
||||
|
||||
user_group_view.set_sync_after_recursive_query(barrier)
|
||||
global BARRIER
|
||||
BARRIER = threading.Barrier(parties=2, timeout=3)
|
||||
t1.start()
|
||||
t2.start()
|
||||
|
||||
@@ -91,12 +138,11 @@ have no control over the scheduler when the barrier timeouts.
|
||||
for t in [t1, t2]:
|
||||
t.join()
|
||||
response = t.response
|
||||
if response is not None and response.status_code == 200:
|
||||
if response is None:
|
||||
succeeded += 1
|
||||
continue
|
||||
|
||||
assert response is not None
|
||||
self.assert_json_error(response, error_messsage)
|
||||
self.assertEqual(response, error_messsage)
|
||||
# Race condition resolution should only allow one thread to succeed
|
||||
self.assertEqual(
|
||||
succeeded,
|
||||
|
||||
@@ -1,65 +0,0 @@
|
||||
import threading
|
||||
from typing import Any, Optional
|
||||
from unittest import mock
|
||||
|
||||
from django.db import OperationalError, transaction
|
||||
from django.http import HttpRequest, HttpResponse
|
||||
|
||||
from zerver.lib.exceptions import JsonableError
|
||||
from zerver.lib.request import REQ, has_request_variables
|
||||
from zerver.lib.response import json_success
|
||||
from zerver.lib.user_groups import access_user_group_by_id
|
||||
from zerver.lib.validator import check_int
|
||||
from zerver.models import UserGroup, UserProfile
|
||||
from zerver.views.user_groups import update_subgroups_of_user_group
|
||||
|
||||
BARRIER: Optional[threading.Barrier] = None
|
||||
|
||||
|
||||
def set_sync_after_recursive_query(barrier: Optional[threading.Barrier]) -> None:
|
||||
global BARRIER
|
||||
BARRIER = barrier
|
||||
|
||||
|
||||
@has_request_variables
|
||||
def dev_update_subgroups(
|
||||
request: HttpRequest,
|
||||
user_profile: UserProfile,
|
||||
user_group_id: int = REQ(json_validator=check_int, path_only=True),
|
||||
) -> HttpResponse:
|
||||
# The test is expected to set up the barrier before accessing this endpoint.
|
||||
assert BARRIER is not None
|
||||
try:
|
||||
with transaction.atomic(), mock.patch(
|
||||
"zerver.lib.user_groups.access_user_group_by_id"
|
||||
) as m:
|
||||
|
||||
def wait_after_recursive_query(*args: Any, **kwargs: Any) -> UserGroup:
|
||||
# When updating the subgroups, we access the supergroup group
|
||||
# only after finishing the recursive query.
|
||||
BARRIER.wait()
|
||||
return access_user_group_by_id(*args, **kwargs)
|
||||
|
||||
m.side_effect = wait_after_recursive_query
|
||||
|
||||
update_subgroups_of_user_group(request, user_profile, user_group_id=user_group_id)
|
||||
except OperationalError as err:
|
||||
msg = str(err)
|
||||
if "deadlock detected" in msg:
|
||||
raise JsonableError("Deadlock detected")
|
||||
else:
|
||||
assert "could not obtain lock" in msg
|
||||
# This error is possible when nowait is set the True, which only
|
||||
# applies to the recursive query on the subgroups. Because the
|
||||
# recursive query fails, this thread must have not waited on the
|
||||
# barrier yet.
|
||||
BARRIER.wait()
|
||||
raise JsonableError("Busy lock detected")
|
||||
except (
|
||||
threading.BrokenBarrierError
|
||||
): # nocoverage # This is only possible when timeout happens or there is a programming error
|
||||
raise JsonableError(
|
||||
"Broken barrier. The tester should make sure that the exact number of parties have waited on the barrier set by the previous immediate set_sync_after_first_lock call"
|
||||
)
|
||||
|
||||
return json_success(request)
|
||||
@@ -10,7 +10,6 @@ from django.urls import path
|
||||
from django.views.generic import TemplateView
|
||||
from django.views.static import serve
|
||||
|
||||
from zerver.lib.rest import rest_path
|
||||
from zerver.views.auth import config_error, login_page
|
||||
from zerver.views.development.cache import remove_caches
|
||||
from zerver.views.development.camo import handle_camo_url
|
||||
@@ -32,7 +31,6 @@ from zerver.views.development.registration import (
|
||||
register_development_realm,
|
||||
register_development_user,
|
||||
)
|
||||
from zerver.views.development.user_groups import dev_update_subgroups
|
||||
|
||||
# These URLs are available only in the development environment
|
||||
|
||||
@@ -100,14 +98,6 @@ urls = [
|
||||
path("external_content/<digest>/<received_url>", handle_camo_url),
|
||||
]
|
||||
|
||||
testing_urls = [
|
||||
rest_path(
|
||||
"testing/user_groups/<int:user_group_id>/subgroups",
|
||||
POST=(dev_update_subgroups, {"intentionally_undocumented"}),
|
||||
),
|
||||
]
|
||||
urls += testing_urls
|
||||
|
||||
v1_api_mobile_patterns = [
|
||||
# This is for the signing in through the devAuthBackEnd on mobile apps.
|
||||
path("dev_fetch_api_key", api_dev_fetch_api_key),
|
||||
|
||||
Reference in New Issue
Block a user