Skip to content

Commit

Permalink
Don't try to use a fixed port in test_server_config_default (#8272)
Browse files Browse the repository at this point in the history
We had a flake regarding this.

(At least on my machine?), linux never randomly assigns an even
numbered port, so probably the flake collided with something external
specifically looking for it?

The find_available_port mechanism is fundamentally racy, of course;
the port could be allocated by someone else immediately, and in
testing I did manage to observe cycle lengths as short as 2 before a
reuse.

Possibly these tests should just have a big hammer
`@retry_failed_test(3)` decorator around them or something, but I am
reluctant to introduce this because I fear we will use it instead of
writing proper tests.

(Most tests don't have as good an excuse for flakiness as needing to
allocate a shared resource in a 16-bit namespace).
  • Loading branch information
msullivan authored Feb 1, 2025
1 parent 0676b5e commit a8c5ae0
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 19 deletions.
21 changes: 4 additions & 17 deletions edb/testbase/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -3002,23 +3002,10 @@ def get_cases_by_shard(cases, selected_shard, total_shards, verbosity, stats):
return _merge_results(cases)


def find_available_port(max_value=None) -> int:
if max_value is None:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
sock.bind(("localhost", 0))
return sock.getsockname()[1]
elif max_value > 1024:
port = max_value
while port > 1024:
try:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
sock.bind(("localhost", port))
return port
except IOError:
port -= 1
raise RuntimeError("cannot find an available port")
else:
raise ValueError("max_value must be greater than 1024")
def find_available_port() -> int:
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as sock:
sock.bind(("localhost", 0))
return sock.getsockname()[1]


def _needs_factoring(weakly):
Expand Down
4 changes: 2 additions & 2 deletions tests/test_server_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -2366,7 +2366,7 @@ async def test_server_config_env_03(self):
"cannot use CONFIGURE INSTANCE in multi-tenant mode",
)
async def test_server_config_default(self):
p1 = tb.find_available_port(max_value=50000)
p1 = tb.find_available_port()
async with tb.start_edgedb_server(
extra_args=["--port", str(p1)]
) as sd:
Expand All @@ -2378,7 +2378,7 @@ async def test_server_config_default(self):
"""),
p1,
)
p2 = tb.find_available_port(p1 - 1)
p2 = tb.find_available_port()
await conn.execute(f"""\
configure instance set listen_port := {p2}
""")
Expand Down

0 comments on commit a8c5ae0

Please sign in to comment.