Skip to content

Add Unit test for AMQP Client Link with max message size = 0 #43582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add Unit test for AMQP Client Link with max message size = 0 #43582

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 70 additions & 0 deletions sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_link.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
from azure.eventhub._pyamqp.link import Link
from azure.eventhub._pyamqp.receiver import ReceiverLink
from azure.eventhub._pyamqp.constants import LinkState
from azure.eventhub._pyamqp.link import Source, Target
from unittest.mock import Mock, patch
from azure.eventhub._pyamqp.constants import LINK_MAX_MESSAGE_SIZE
import pytest


Expand Down Expand Up @@ -84,6 +87,73 @@ def test_receive_transfer_frame_multiple():
link._incoming_transfer(transfer_frame_two)
assert link.current_link_credit == 1

def test_max_message_size_negotiation_with_client_unlimited():
"""
Test AMQP attach frame negotiation where client sends max_message_size=0 (unlimited)
and server responds with its limit (20MB), resulting in final size of 20MB.

Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring has trailing whitespace on line 94 and an unnecessary blank line 95. Remove the trailing whitespace and the blank line before the closing triple quotes.

Suggested change

Copilot uses AI. Check for mistakes.
"""
mock_session = Mock()
mock_connection = Mock()
mock_session._connection = mock_connection

SERVER_MAX_MESSAGE_SIZE = 20 * 1024 * 1024

link = Link(
mock_session,
3,
name="test_link",
role=False, # Sender role
source_address="test_source",
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test uses hardcoded string values (test_source, test_target, test_link) that are duplicated in both the Link instantiation and the mock attach frame. Consider defining these as constants at the beginning of the test function to improve maintainability and reduce the risk of inconsistencies.

Copilot uses AI. Check for mistakes.
target_address="test_target",
network_trace=False,
network_trace_params={},
max_message_size=LINK_MAX_MESSAGE_SIZE
)

# Verifying that client sends 0 (unlimited) in attach frame
assert link.max_message_size == 0, f"Expected client max_message_size=0, got {link._max_message_size}"
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion message references link._max_message_size (private attribute) but the assertion checks link.max_message_size (public property). The error message should reference the same attribute being tested: change {link._max_message_size} to {link.max_message_size}.

Suggested change
assert link.max_message_size == 0, f"Expected client max_message_size=0, got {link._max_message_size}"
assert link.max_message_size == 0, f"Expected client max_message_size=0, got {link.max_message_size}"

Copilot uses AI. Check for mistakes.
# Simulating server's attach response with 20MB limit, Mock incoming attach frame from server
mock_attach_frame = [
"test_link",
3,
False,
0,
1,
Source(address="test_source"),
Target(address="test_target"),
None,
False,
None,
20 * 1024 * 1024,
None,
None,
None, ]
Comment on lines +118 to +131
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mock attach frame list lacks comments explaining what each positional element represents. Add inline comments describing each field (e.g., name, handle, role, snd_settle_mode, rcv_settle_mode, source, target, unsettled, incomplete_unsettled, initial_delivery_count, max_message_size, offered_capabilities, desired_capabilities, properties) to improve test readability and maintainability.

Suggested change
"test_link",
3,
False,
0,
1,
Source(address="test_source"),
Target(address="test_target"),
None,
False,
None,
20 * 1024 * 1024,
None,
None,
None, ]
"test_link", # name
3, # handle
False, # role (False = sender, True = receiver)
0, # snd_settle_mode
1, # rcv_settle_mode
Source(address="test_source"),# source
Target(address="test_target"),# target
None, # unsettled
False, # incomplete_unsettled
None, # initial_delivery_count
20 * 1024 * 1024, # max_message_size
None, # offered_capabilities
None, # desired_capabilities
None, # properties
]

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are multiple spaces before the closing bracket. This appears to be inconsistent whitespace that should be removed for cleaner formatting.

Suggested change
None, ]
None, ]

Copilot uses AI. Check for mistakes.

# Testing _outgoing_attach()
with patch.object(link, '_outgoing_attach') as mock_outgoing_attach:
# Trigger outgoing attach
link.attach()

# Verifying _outgoing_attach was called
assert mock_outgoing_attach.called, "_outgoing_attach should have been called during attach()"

# Get the attach frame that would be sent to server
call_args = mock_outgoing_attach.call_args
if call_args and call_args[0]:
attach_frame = call_args[0][0]
assert attach_frame.max_message_size == 0, f"Expected client to send max_message_size=0, got {attach_frame.max_message_size}"

# Testing _incoming_attach()
with patch.object(link, '_outgoing_attach') as mock_outgoing_response:

# Calling _incoming_attach to process server's response
link._incoming_attach(mock_attach_frame)

expected_final_size = SERVER_MAX_MESSAGE_SIZE
# Verifying remote_max_message_size is set correctly
assert link.remote_max_message_size == expected_final_size, \
f"Expected remote_max_message_size={SERVER_MAX_MESSAGE_SIZE}, got {link.remote_max_message_size}"
Copy link

Copilot AI Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion message uses SERVER_MAX_MESSAGE_SIZE directly instead of the expected_final_size variable that was just defined. For consistency and clarity, the message should reference expected_final_size to match what's being tested.

Suggested change
f"Expected remote_max_message_size={SERVER_MAX_MESSAGE_SIZE}, got {link.remote_max_message_size}"
f"Expected remote_max_message_size={expected_final_size}, got {link.remote_max_message_size}"

Copilot uses AI. Check for mistakes.

def test_receive_transfer_continuation_frame():
session = None
Expand Down