Skip to content

Commit

Permalink
Enable py3 server and client_server tests in auto_migrate mode
Browse files Browse the repository at this point in the history
Summary:
These tests mostly don't work because the thrift py3 server library doesn't accept thrift python server interfaces, which has always been very low pri to fix because the all the ways of setting up thrift py3 services also accept thrift python services, meaning that this only breaks for people setting up raw py3 services.

Enabling it just for the sake of better tracking.

The bad unicode test is odd though, because it seems that rather than throwing a nice `UnicodeDecodeError` like thrift py3 does, a very ugly `IndexError` gets doesn't get caught and causes that test to fail even with `brokenInAutoMigrate()`. Worth investigating, but for now I'm just bypassing that test.

Reviewed By: yoney

Differential Revision: D68870475

fbshipit-source-id: eafa2022d3f7024eabdd37788e6087611fd7b831
  • Loading branch information
Filip Francetic authored and facebook-github-bot committed Jan 30, 2025
1 parent e1ccd4f commit 3611f9c
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,9 @@
ClientMetadataTestingServiceInterface,
TestingServiceInterface,
)
from testing.thrift_types import easy as Python_easy
from testing.types import Color, easy, HardError
from thrift.lib.py3.test.auto_migrate.auto_migrate_util import brokenInAutoMigrate
from thrift.py3.client import ClientType, get_client
from thrift.py3.common import Priority, Protocol, RpcOptions
from thrift.py3.exceptions import ApplicationError
Expand Down Expand Up @@ -139,6 +141,7 @@ class ClientServerTests(unittest.TestCase):
These are tests where a client and server talk to each other
"""

@brokenInAutoMigrate()
# pyre-fixme[56]: Argument `sys.version_info[slice(None, 2, None)] < (3, 7)` to
# decorator factory `unittest.skipIf` could not be resolved in a global scope.
@unittest.skipIf(sys.version_info[:2] < (3, 7), "Requires py3.7")
Expand Down Expand Up @@ -180,6 +183,7 @@ async def outside_context_test() -> None:

loop.run_until_complete(outside_context_test())

@brokenInAutoMigrate()
def test_rpc_headers(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -195,6 +199,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_client_resolve(self) -> None:
loop = asyncio.get_event_loop()
hostname = socket.gethostname()
Expand All @@ -212,6 +217,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_unframed_binary(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -231,6 +237,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_framed_deprecated(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -249,6 +256,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_framed_compact(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -267,6 +275,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_server_localhost(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -280,6 +289,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_unix_socket(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -293,6 +303,7 @@ async def inner_test(dir: Path) -> None:
with tempfile.TemporaryDirectory() as tdir:
loop.run_until_complete(inner_test(Path(tdir)))

@brokenInAutoMigrate()
def test_no_client_aexit(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -309,6 +320,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_client_aexit_no_await(self) -> None:
"""
This actually handles the case if __aexit__ is not awaited
Expand All @@ -330,6 +342,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_no_client_no_aenter(self) -> None:
"""
This covers if aenter was canceled since those two are the same really
Expand All @@ -346,6 +359,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_derived_service(self) -> None:
"""
This tests calling methods from a derived service
Expand All @@ -366,21 +380,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

def test_non_utf8_exception_message(self) -> None:
loop = asyncio.get_event_loop()

async def inner_test() -> None:
async with TestServer(handler=CppHandler()) as sa:
ip, port = sa.ip, sa.port
assert ip and port
async with get_client(TestingService, host=ip, port=port) as client:
with self.assertRaises(HardError):
await client.hard_error(True)
with self.assertRaises(UnicodeDecodeError):
await client.hard_error(False)

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_renamed_func(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -393,6 +393,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_queue_timeout(self) -> None:
"""
This tests whether queue timeout functions properly.
Expand Down Expand Up @@ -435,6 +436,7 @@ async def clients_run(server: TestServer) -> None:

loop.run_until_complete(clients_run(testing))

@brokenInAutoMigrate()
def test_cancelled_task(self) -> None:
"""
This tests whether cancelled tasks are handled properly.
Expand Down Expand Up @@ -463,6 +465,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_request_with_default_rpc_options(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -478,6 +481,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_request_with_specified_rpc_options(self) -> None:
loop = asyncio.get_event_loop()

Expand All @@ -497,6 +501,22 @@ async def inner_test() -> None:
loop.run_until_complete(inner_test())


class Utf8Test(unittest.IsolatedAsyncioTestCase):
@brokenInAutoMigrate()
async def test_non_utf8_exception_message(self) -> None:
# something about non-utf8 exceptions seems extra broken, I need to skip the test entirely
if Python_easy is easy:
self.assertTrue(False)
async with TestServer(handler=CppHandler()) as sa:
ip, port = sa.ip, sa.port
assert ip and port
async with get_client(TestingService, host=ip, port=port) as client:
with self.assertRaises(HardError):
await client.hard_error(True)
with self.assertRaises(UnicodeDecodeError):
await client.hard_error(False)


class StackHandler(StackServiceInterface):
async def add_to(self, lst: Sequence[int], value: int) -> Sequence[int]:
return [x + value for x in lst]
Expand Down Expand Up @@ -532,6 +552,7 @@ class ClientStackServerTests(unittest.TestCase):
These are tests where a client and server(stack_arguments) talk to each other
"""

@brokenInAutoMigrate()
def test_server_localhost(self) -> None:
loop = asyncio.get_event_loop()

Expand Down Expand Up @@ -576,6 +597,7 @@ async def getMetadaField(self, key: str) -> str:


class ClientMetadataTestingServiceTests(unittest.TestCase):
@brokenInAutoMigrate()
def test_client_metadata(self) -> None:
loop = asyncio.get_event_loop()
hostname: str = socket.gethostname()
Expand Down Expand Up @@ -606,6 +628,7 @@ async def inner_test() -> None:

loop.run_until_complete(inner_test())

@brokenInAutoMigrate()
def test_call_get_metadata_field_with_invalid_key_should_return_empty_field(
self,
) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

from testing.services import TestingServiceInterface
from testing.types import Color, easy
from thrift.lib.py3.test.auto_migrate.auto_migrate_util import brokenInAutoMigrate
from thrift.py3.server import getServiceName, SocketAddress, ThriftServer
from thrift.py3.test.is_overload.helper import OverloadTestHelper

Expand Down Expand Up @@ -96,10 +97,12 @@ async def inner() -> None:

loop.run_until_complete(inner())

@brokenInAutoMigrate()
def test_get_service_name(self) -> None:
h = Handler()
self.assertEqual(getServiceName(h), "TestingService")

@brokenInAutoMigrate()
def test_get_address(self) -> None:
loop = asyncio.get_event_loop()
coro = self.get_address(loop)
Expand All @@ -113,6 +116,7 @@ async def get_address(self, loop: asyncio.AbstractEventLoop) -> SocketAddress:
await serve_task
return addy

@brokenInAutoMigrate()
def test_annotations(self) -> None:
annotations = TestingServiceInterface.annotations
self.assertIsInstance(annotations, types.MappingProxyType)
Expand All @@ -137,6 +141,7 @@ def test_unittest_call_renamed_func(self) -> None:
ret = loop.run_until_complete(h.renamed_func(True))
self.assertTrue(ret)

@brokenInAutoMigrate()
def test_server_manipulate_config(self) -> None:
MAX_REQUESTS = 142
MAX_CONNECTIONS = 132
Expand Down Expand Up @@ -170,19 +175,22 @@ def test_server_manipulate_config(self) -> None:
server.set_quick_exit_on_shutdown_timeout(True)
self.assertTrue(server.get_quick_exit_on_shutdown_timeout())

@brokenInAutoMigrate()
def test_server_get_stats(self) -> None:
server = ThriftServer(Handler(), port=0)

active_requests = server.get_active_requests()
self.assertGreaterEqual(active_requests, 0)
self.assertLess(active_requests, 10)

@brokenInAutoMigrate()
def test_set_is_overloaded(self) -> None:
helper = OverloadTestHelper(Handler(), 0)
helper.set_is_overload()
self.assertTrue(helper.check_overload("overloaded_method"))
self.assertFalse(helper.check_overload("not_overloaded_method"))

@brokenInAutoMigrate()
def test_lifecycle_hooks(self) -> None:
async def inner() -> None:
handler = Handler()
Expand Down

0 comments on commit 3611f9c

Please sign in to comment.