You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/09/30 21:04:14 UTC
[GitHub] [trafficserver] ywkaras opened a new pull request #7232: Fix lua_states_stats Au test.
ywkaras opened a new pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
traeak commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498403540
##########
File path: tests/gold_tests/pluginTest/lua/gold/metrics.gold
##########
@@ -0,0 +1,6 @@
+plugin.lua.global.states 4
+plugin.lua.global.gc_bytes 249856
Review comment:
of this whole file the only piece that probably matters is the
plugin.lua.global.states 4
The rest of the numbers don't really matter. Actually eliminating the gold file and just looking for the above to be there and '4' would be a better way to do it.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498505006
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
The argument against is that we want to minimize regression runtime. Reducing unnecessary copying may reduce runtime.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] randall commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498320773
##########
File path: tests/gold_tests/pluginTest/lua/gold/metrics.gold
##########
@@ -0,0 +1,6 @@
+plugin.lua.global.states 4
+plugin.lua.global.gc_bytes 249856
Review comment:
will gc_bytes always be this value? across different platforms?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r505659293
##########
File path: tests/gold_tests/pluginTest/lua/metrics.sh
##########
@@ -0,0 +1,31 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+N=60
+while (( N > 0 ))
+do
+ rm -f metrics.out metrics.txt
+ traffic_ctl metric match lua > metrics.out
Review comment:
Hmm, I don't know enough about metrics generally to know if there is a risk that other metrics would appear in the output in a pseudo-random fashion. I did not see that in my testing.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-701648005
Fixes https://github.com/apache/trafficserver/issues/6846 .
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras removed a comment on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras removed a comment on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-702386473
[approve ci autest]
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r572387316
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -23,16 +23,25 @@
Test.SkipUnless(
Condition.PluginExists('tslua.so'),
)
-Test.SkipIf(Condition.true("Test cannot deterministically wait until the stats appear"))
Test.ContinueOnFail = True
# Define default ATS
server = Test.MakeOriginServer("server")
-ts = Test.MakeATSProcess("ts", command="traffic_manager", select_ports=True)
+# It is necessary to redirect stderr to a file so it will be available for examination by a test run.
+ts = Test.MakeATSProcess(
+ "ts", command="traffic_manager 2> " + Test.RunDirectory + "/ts.stderr.txt", select_ports=True
Review comment:
We already have this text being grabbed in the Streams object. We should have examples test using later for a given test
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r572393271
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -23,16 +23,25 @@
Test.SkipUnless(
Condition.PluginExists('tslua.so'),
)
-Test.SkipIf(Condition.true("Test cannot deterministically wait until the stats appear"))
Test.ContinueOnFail = True
# Define default ATS
server = Test.MakeOriginServer("server")
-ts = Test.MakeATSProcess("ts", command="traffic_manager", select_ports=True)
+# It is necessary to redirect stderr to a file so it will be available for examination by a test run.
+ts = Test.MakeATSProcess(
+ "ts", command="traffic_manager 2> " + Test.RunDirectory + "/ts.stderr.txt", select_ports=True
Review comment:
The lifecycle_stats.sh script needs to read the stderr output from traffic_manager. I tried reading it from the stderr file in _output for the ts process, but the output the script needs gets buffered in memory. In any case, not good to hardcode the sandbox tree structure into tests.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] brbzull0 commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
brbzull0 commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r505648834
##########
File path: tests/gold_tests/pluginTest/lua/metrics.sh
##########
@@ -0,0 +1,31 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+N=60
+while (( N > 0 ))
+do
+ rm -f metrics.out metrics.txt
+ traffic_ctl metric match lua > metrics.out
Review comment:
would it make more sense to just get all the metrics by explicitly mention their names? can occur that we get more metrics than the ones you defined in the `metrics.gold`? probably *not* as this is isolated in this test(worth asking). In any case if you request for a record that does not exist you will get an error out of the `traffic_ctl` (in case you request `traffic_ctl metric get a b c`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-703826223
dragon512/jason will replace this with a different PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498391331
##########
File path: tests/gold_tests/pluginTest/lua/gold/metrics.gold
##########
@@ -0,0 +1,6 @@
+plugin.lua.global.states 4
+plugin.lua.global.gc_bytes 249856
Review comment:
Good point, the original test didn't look for specific numbers. I'm not at all familiar with the Lua plugin. I just addressed the problem of getting Au test to wait for stats accumulation and logging output. Is @traeak still working on ATS?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498513689
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
Hmm but the sandbox always has symlinks outside the sanbox. So it's only reproducible if you maintain dependencies outside the sandbox in any case.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] traeak commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
traeak commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498403540
##########
File path: tests/gold_tests/pluginTest/lua/gold/metrics.gold
##########
@@ -0,0 +1,6 @@
+plugin.lua.global.states 4
+plugin.lua.global.gc_bytes 249856
Review comment:
of this whole file the only piece that probably matters is the
plugin.lua.global.states 4
The rest of the numbers don't really matter. Actually eliminating the gold file and just looking for the above to be there and '4' would be a better way to do it. gold files can be finnicky.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras merged pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras merged pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-772704629
@dragon512 I removed all usage of Test.TestDirectory.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498503042
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
Why?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-769450692
@ywkaras is going to fix the issue @dragon512 pointed out.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-702386473
[approve ci autest]
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498501457
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
Don't run stuff in the test directory. Copy this to the sandbox and run it
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
ps.Env = ts.Env
ps.ReturnCode = 0
-ps.Streams.stdout = "gold/metrics.stdout.gold"
tr.StillRunningAfter = ts
# 3 Test - Check for developer lifecycle stats
tr = Test.AddTestRun("Check for lifecycle stats")
ps = tr.Processes.Default # alias
-ps.Command = "traffic_ctl plugin msg ts_lua print_stats"
+ps.Command = "bash -c '" + Test.TestDirectory + "/lifecycle_stats.sh " + Test.TestDirectory + "'"
Review comment:
Same here.. One should almost never need to call Test.TestDirectory.
Just add someplace a `Test.Copy('lifecycle_stats.sh")`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] ywkaras commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
ywkaras commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-769451181
Per ATS team meeting, we will go ahead an try to merge this, then @jkenny can suggest improvements.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] dragon512 commented on a change in pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
dragon512 commented on a change in pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#discussion_r498506297
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
This is not a time suck. it is about reproducibility
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
Because one can't replay the sandbox
##########
File path: tests/gold_tests/pluginTest/lua/lua_states_stats.test.py
##########
@@ -67,36 +71,27 @@
ps.Env = ts.Env
ps.ReturnCode = 0
ps.Streams.stdout.Content = Testers.ContainsExpression("proxy.config.plugin.lua.max_states: 4", "expected 4 states")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 1 Test - Exercise lua script
tr = Test.AddTestRun("Lua hello")
ps = tr.Processes.Default # alias
ps.Command = curl_and_args + ' http://hello/hello'
-ps.TimeOut = 5
ps.ReturnCode = 0
ps.Streams.stderr.Content = Testers.ContainsExpression("Hello, World", "hello world content")
-tr.TimeOut = 5
tr.StillRunningAfter = ts
# 2 Test - Check for metrics
tr = Test.AddTestRun("Check for metrics")
-tr.DelayStart = 15 # 5s lag on metrics to update
-tr.TimeOut = 5
ps = tr.Processes.Default # alias
-ps.Env = ts.Env
-ps.Command = "traffic_ctl metric match lua"
+ps.Command = "bash -c '" + Test.TestDirectory + "/metrics.sh " + Test.TestDirectory + "'"
Review comment:
The time suck to I should clarify is all the sleeps and a lack of a way to detect that the metrics are up-to-date
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [trafficserver] bryancall commented on pull request #7232: Fix lua_states_stats Au test.
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7232:
URL: https://github.com/apache/trafficserver/pull/7232#issuecomment-769449567
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org