Yicong-Huang commented on code in PR #8276:
URL: https://github.com/apache/texera/pull/8276#discussion_r3936582335
##########
amber/src/test/python/core/architecture/packaging/test_output_manager.py:
##########
@@ -666,6 +666,29 @@ def
test_get_port_ids_returns_added_ports_in_insertion_order(self, output_manage
output_manager.add_output_port(port_b, MagicMock())
assert output_manager.get_port_ids() == [port_a, port_b]
+ def test_get_port_honors_the_requested_port_id(self, output_manager):
+ port_a = PortIdentity(id=0, internal=False)
+ port_b = PortIdentity(id=1, internal=False)
+ schema_a = MagicMock(name="schema_a")
+ schema_b = MagicMock(name="schema_b")
+ output_manager.add_output_port(port_a, schema_a)
+ output_manager.add_output_port(port_b, schema_b)
+ assert output_manager.get_port(port_b).get_schema() is schema_b
+
+ def test_get_port_without_an_id_uses_the_first_port(self, output_manager):
+ schema = MagicMock(name="schema")
+ output_manager.add_output_port(PortIdentity(id=0), schema)
+ assert output_manager.get_port().get_schema() is schema
Review Comment:
With one port registered, "first" and "only" are the same thing — an
implementation that returned the *last* registered port would pass this
unchanged. The insertion-order choice is what `tuple_to_frame` relies on
(`output_manager.py:361`, `:363`), so it is worth pinning. Registering a second
port makes the selection observable, the way the case above does.
```suggestion
port_a = PortIdentity(id=0, internal=False)
port_b = PortIdentity(id=1, internal=False)
schema_a = MagicMock(name="schema_a")
output_manager.add_output_port(port_a, schema_a)
output_manager.add_output_port(port_b, MagicMock(name="schema_b"))
assert output_manager.get_port().get_schema() is schema_a
```
--
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]
