Copilot commented on code in PR #13012:
URL: https://github.com/apache/trafficserver/pull/13012#discussion_r2976632510
##########
tests/gold_tests/logging/sigusr2.test.py:
##########
@@ -130,9 +130,9 @@ def get_sigusr2_signal_command(self):
# Configure the signaling of SIGUSR2 to traffic_server.
tr1.Processes.Default.Command = diags_test.get_sigusr2_signal_command()
tr1.Processes.Default.Return = 0
-tr1.Processes.Default.Ready = When.FileExists(diags_test.diags_log)
-
-# Configure process order.
+# Configure process order: ts starts first, then rotate moves diags.log,
+# then Default sends SIGUSR2. No Ready condition needed on Default since the
+# StartBefore chain already ensures ts is fully started before rotate runs.
tr1.Processes.Default.StartBefore(rotate_diags_log)
rotate_diags_log.StartBefore(diags_test.ts)
Review Comment:
The comment says the StartBefore chain ensures TS is "fully started", but
StartBefore only enforces launch ordering; it doesn’t guarantee `diags.log`
exists before `mv` runs. With the Ready condition removed from Default,
`rotate_diags_log` can still race TS startup and fail if `diags.log` hasn’t
been created yet. Consider moving the file-exists gating to the rotate step
(e.g., via a `ready=` condition on the StartBefore edge or by making the rotate
command wait/retry) and/or adjust the comment to avoid implying readiness
guarantees.
##########
tests/gold_tests/timeout/ssl-delay-server.cc:
##########
@@ -199,8 +204,10 @@ main(int argc, char *argv[])
for (;;) {
sfd = accept(listenfd, (struct sockaddr *)nullptr, nullptr);
if (sfd <= 0) {
Review Comment:
`accept()` returns `-1` on error, but can legally return fd `0` on success
(e.g., if stdin is closed). The `if (sfd <= 0)` check will treat a valid
connection as failure and exit, potentially reintroducing flakiness. Use `sfd <
0` for the error check, keeping the EINTR retry logic the same.
```suggestion
if (sfd < 0) {
```
--
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]
