Lunderberg commented on a change in pull request #10581:
URL: https://github.com/apache/tvm/pull/10581#discussion_r831104847
##########
File path: tests/python/contrib/test_hexagon/conftest.py
##########
@@ -59,27 +64,113 @@ def requires_hexagon_toolchain(*args):
@tvm.testing.fixture
-def android_serial_number() -> str:
- return os.getenv(ANDROID_SERIAL_NUMBER, default=None)
+def android_serial_number() -> Optional[str]:
+ serial = os.getenv(ANDROID_SERIAL_NUMBER, default="")
+ # Setting ANDROID_SERIAL_NUMBER to an empty string should be
+ # equivalent to having it unset.
+ if not serial.strip():
+ serial = None
+ return serial
@tvm.testing.fixture
def tvm_tracker_host() -> str:
return os.getenv(TVM_TRACKER_HOST, default=None)
[email protected]
+# NOTE on server ports:
+# These tests use different port numbers for the RPC server (7070 + ...).
+# The reason is that an RPC session cannot be gracefully closed without
+# triggering TIME_WAIT state on the server socket. This prevents another
+# server to bind to the same port until the wait time elapses.
+
+listen_port_min = 2000 # Well above the privileged ports (1024 or lower)
+listen_port_max = 9000 # Below the search range end (port_end=9199) of RPC
server
+previous_port = [None]
+
+
+def get_free_port():
+ # https://stackoverflow.com/a/52872579/2689797
+ def is_port_in_use(port: int) -> bool:
+ with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
+ return s.connect_ex(("localhost", port)) == 0
+
+ if previous_port[0] is None:
+ port = random.randint(listen_port_min, listen_port_max)
+ else:
+ port = previous_port[0] + 1
+
+ while is_port_in_use(port):
+ port = port + 1 if port < listen_port_max else listen_port_min
+
+ previous_port[0] = port
+ return port
+
+
[email protected](scope="session")
def tvm_tracker_port() -> int:
port = os.getenv(TVM_TRACKER_PORT, default=None)
- port = int(port) if port else None
+ port = int(port) if port else get_free_port()
return port
[email protected](scope="session")
+def tvm_tracker(tvm_tracker_port):
Review comment:
Thank you, and I hadn't accounted for that. It looks like it is handled
similarly in `tests/scripts/task_python_hexagon_simulator.sh`, with the script
starting the tracker before running pytest. I still like having the tracker
start from the `pytest` call, since that minimizes the amount of local setup
needed to reproduce a CI failure, but I can see how it would be useful to avoid
repeated setup/teardown of the tracker in CI.
I like the idea of choosing whether `pytest` starts a tracker, though I
think I'll implement it based on the presence of the `TVM_TRACKER_HOST` and
`TVM_TRACKER_PORT` environment variables. This would avoid a potential failure
mode from using a server query in a multi-user system, where somebody else's
tracker is mistaken for one's own.
--
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]
