bneradt commented on code in PR #12886:
URL: https://github.com/apache/trafficserver/pull/12886#discussion_r2805577705
##########
tests/gold_tests/thread_config/thread_config.test.py:
##########
@@ -39,7 +39,7 @@
TS_ROOT = ts.Env['TS_ROOT']
tr.Processes.Default.Command = f'{sys.executable} check_threads.py -p
{TS_ROOT} -e 1 -a 0 -t 1 -c 1'
tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.StartBefore(ts)
+tr.Processes.Default.StartBefore(ts, ready=When.PortOpen(ts.Variables.port))
Review Comment:
This seems incorrect. ATS is not ready when the port is open...ATS opens the
port and then does a bunch of work afterwards before it is ready. Waiting on
PortOpen used to cause issues in the past. This PortOpen ready condition was
intentionally changed to wait upon the log message which gives a more reliable
marker for ATS being ready. This is why we have the ready condition tied to the
log output in trafficserver.text.ext:
```python
p.Ready = When.FileContains(p.Disk.diags_log.AbsPath, "NOTE: Traffic
Server is fully initialized")
```
Am I missing something? If I am, please explain carefully why awaiting on
PortOpen is better than waiting upon the "Traffic Server is fully initialized"
message.
##########
tests/gold_tests/post/post-early-return.test.py:
##########
@@ -83,23 +83,23 @@
# The third case has an explicit multi-second sleep which ensures the early
response path is exercised
test_run = Test.AddTestRun("http1.1 Post with small body early return")
test_run.Processes.Default.StartBefore(Test.Processes.ts)
-test_run.Processes.Default.StartBefore(server1)
+test_run.Processes.Default.StartBefore(server1,
ready=When.PortOpen(Test.Variables.upstream_port1))
Review Comment:
Rather than associate the ready with StartBefore, I think it would be
preferable to associate it with the server1, server2, etc., processes
themselves. That way the Ready condition is more tightly tied to the process
rather than a function call associated with the process. Thus:
```python
server1.Ready = When.PortOpen(Test.Variables.upstream_port1)
```
This is how things are done, for instance, for the MakeATSProcess Processes
in trafficserver.test.ext.
--
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]
