SameerMesiah97 commented on code in PR #70052:
URL: https://github.com/apache/airflow/pull/70052#discussion_r3609642749
##########
airflow-core/tests/unit/models/test_connection.py:
##########
@@ -540,3 +540,26 @@ def test_get_conn_id_to_team_name_mapping(self,
testing_team: Team, session: Ses
"test_conn2": None,
}
clear_db_connections()
+
+ def test_port_validation(self):
+ """Test that Connection model validates the port field correctly."""
+ # Valid ports
+ assert Connection(conn_id="test_port_1", port=80).port == 80
+ assert Connection(conn_id="test_port_2", port="8080").port == 8080
+ assert Connection(conn_id="test_port_3", port=None).port is None
+ assert Connection(conn_id="test_port_4", port="").port is None
+ assert Connection(conn_id="test_port_5", port=" ").port is None
+
+ # Invalid ports - out of range
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_1", port=70000)
+
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_2", port=0)
+
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_3", port=-80)
+
+ # Invalid ports - type errors
+ with pytest.raises(ValueError, match="Expected integer value for
`port`"):
+ Connection(conn_id="test_port_fail_4", port="invalid_port")
Review Comment:
This test could be better as well. Please see the below:
```
@pytest.mark.parametrize(
("port", "expected"),
[
pytest.param(1, 1, id="min_port"),
pytest.param(80, 80, id="valid_port"),
pytest.param(65535, 65535, id="max_port"),
pytest.param("8080", 8080, id="string_port"),
pytest.param(None, None, id="none"),
pytest.param("", None, id="empty_string"),
pytest.param(" ", None, id="whitespace"),
],
)
def test_port_validation(self, port, expected):
"""Test that Connection model validates the port field correctly."""
assert Connection(conn_id="test", port=port).port == expected
@pytest.mark.parametrize(
"port",
[
pytest.param(70000, id="too_large"),
pytest.param(0, id="zero"),
pytest.param(-80, id="negative"),
],
)
def test_port_validation_out_of_range(self, port):
"""Test that out-of-range ports are rejected."""
with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
Connection(conn_id="test", port=port)
@pytest.mark.parametrize(
"port",
[
pytest.param("invalid_port", id="invalid_string"),
],
)
def test_port_validation_invalid_type(self, port):
"""Test that invalid port types are rejected."""
with pytest.raises(ValueError, match="Expected integer value for
`port`"):
Connection(conn_id="test", port=port)
```
##########
task-sdk/tests/task_sdk/definitions/test_connection.py:
##########
@@ -421,3 +421,41 @@ def test_from_uri_roundtrip(self):
original_extra = json.loads(conn_from_original.extra)
roundtrip_extra = json.loads(conn_from_roundtrip.extra)
assert original_extra == roundtrip_extra
+
+
+class TestConnectionPortValidation:
+ """Test connection port validation in the task-sdk."""
+
+ def test_port_validation(self):
+ """Test that Connection model validates the port field correctly."""
+ # Valid ports
+ assert Connection(conn_id="test_port_1", port=80).port == 80
+ assert Connection(conn_id="test_port_2", port="8080").port == 8080
+ assert Connection(conn_id="test_port_3", port=None).port is None
+
+ # Invalid ports - out of range
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_1", port=70000)
+
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_2", port=0)
+
+ with pytest.raises(ValueError, match="The `port` field must be a value
between 1 and 65535"):
+ Connection(conn_id="test_port_fail_3", port=-80)
+
+ # Invalid ports - type errors
+ with pytest.raises(ValueError, match="Expected integer value for
`port`"):
+ Connection(conn_id="test_port_fail_4", port="invalid_port")
+
+ def test_from_uri_port_validation(self):
+ """Test that Connection.from_uri validates the port field correctly."""
+ # Valid port from URI
+ assert Connection.from_uri("postgres://host:5432/db",
conn_id="test").port == 5432
+
+ # Invalid port from URI - out of range
+ with pytest.raises(ValueError, match="Port out of range 0-65535"):
+ Connection.from_uri("postgres://host:70000/db", conn_id="test")
+
+ # Invalid port from URI - type/invalid format
+ with pytest.raises(ValueError, match="Port could not be cast to
integer value"):
+ Connection.from_uri("postgres://host:abc/db", conn_id="test")
Review Comment:
The tests could be better. Boundary and whitespace coverage is missing
amongst other things. I would suggest the below:
```
class TestConnectionPortValidation:
"""Test connection port validation in the task-sdk."""
@pytest.mark.parametrize(
("port", "expected"),
[
pytest.param(1, 1, id="min_port"),
pytest.param(80, 80, id="valid_port"),
pytest.param("8080", 8080, id="string_port"),
pytest.param(65535, 65535, id="max_port"),
pytest.param(None, None, id="none"),
pytest.param("", None, id="empty_string"),
pytest.param(" ", None, id="whitespace"),
],
)
def test_valid_port(self, port, expected):
"""Test valid port values."""
assert Connection(conn_id="test", port=port).port == expected
@pytest.mark.parametrize(
"port",
[
pytest.param(0, id="zero"),
pytest.param(-80, id="negative"),
pytest.param(70000, id="too_large"),
],
)
def test_invalid_port_range(self, port):
"""Test ports outside the valid network port range."""
with pytest.raises(ValueError, match="The `port` field must be a
value between 1 and 65535"):
Connection(conn_id="test", port=port)
@pytest.mark.parametrize(
"port",
[
pytest.param("invalid_port", id="invalid_string"),
],
)
def test_invalid_port_type(self, port):
"""Test non-integer port values."""
with pytest.raises(ValueError, match="Expected integer value for
`port`"):
Connection(conn_id="test", port=port)
def test_from_uri_port_validation(self):
"""Test that Connection.from_uri validates the port field
correctly."""
assert Connection.from_uri("postgres://host:5432/db",
conn_id="test").port == 5432
with pytest.raises(ValueError, match="Port out of range 0-65535"):
Connection.from_uri("postgres://host:70000/db", conn_id="test")
with pytest.raises(ValueError, match="Port could not be cast to
integer value"):
Connection.from_uri("postgres://host:abc/db", conn_id="test")
```
--
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]
