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 2021/03/28 22:54:27 UTC

[GitHub] [trafficserver] randall opened a new pull request #7639: Remove reference to client.verify.server from tests

randall opened a new pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639


   Option originally removed in 9f15e7750030d298078ab43d62534d331c15d63c


-- 
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 #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#discussion_r603381233



##########
File path: tests/gold_tests/tls_hooks/tls_hooks18.test.py
##########
@@ -56,23 +54,23 @@
 
 Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=2')
 
-tr = Test.AddTestRun("Test two client hello hooks")
+tr=Test.AddTestRun("Test two client hello hooks")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.StillRunningAfter = ts
-tr.StillRunningAfter = server
-tr.Processes.Default.Command = 'curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
-tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = "gold/preaccept-1.gold"
+tr.StillRunningAfter=ts
+tr.StillRunningAfter=server
+tr.Processes.Default.Command='curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode=0
+tr.Processes.Default.Streams.stdout="gold/preaccept-1.gold"

Review comment:
       agreed! This was the committed output from autopep8 target.




-- 
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 pull request #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
randall commented on pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#issuecomment-809012217


   I also went a bit overboard and removed `proxy.config.ssl.server.cipher_suite` where it looked like it was copy pasta (and tests still pass). I can pull that change out separately if preferred


-- 
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 #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
randall commented on a change in pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#discussion_r603412517



##########
File path: tests/gold_tests/tls_hooks/tls_hooks18.test.py
##########
@@ -56,23 +54,23 @@
 
 Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=2')
 
-tr = Test.AddTestRun("Test two client hello hooks")
+tr=Test.AddTestRun("Test two client hello hooks")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.StillRunningAfter = ts
-tr.StillRunningAfter = server
-tr.Processes.Default.Command = 'curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
-tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = "gold/preaccept-1.gold"
+tr.StillRunningAfter=ts
+tr.StillRunningAfter=server
+tr.Processes.Default.Command='curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode=0
+tr.Processes.Default.Streams.stdout="gold/preaccept-1.gold"

Review comment:
       good find! How did this test pass?




-- 
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 merged pull request #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
randall merged pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639


   


-- 
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] bneradt commented on a change in pull request #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#discussion_r603411904



##########
File path: tests/gold_tests/tls_hooks/tls_hooks18.test.py
##########
@@ -56,23 +54,23 @@
 
 Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=2')
 
-tr = Test.AddTestRun("Test two client hello hooks")
+tr=Test.AddTestRun("Test two client hello hooks")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.StillRunningAfter = ts
-tr.StillRunningAfter = server
-tr.Processes.Default.Command = 'curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
-tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = "gold/preaccept-1.gold"
+tr.StillRunningAfter=ts
+tr.StillRunningAfter=server
+tr.Processes.Default.Command='curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode=0
+tr.Processes.Default.Streams.stdout="gold/preaccept-1.gold"

Review comment:
       Here you go: #7643




-- 
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] bneradt commented on a change in pull request #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#discussion_r603372799



##########
File path: tests/gold_tests/tls_hooks/tls_hooks18.test.py
##########
@@ -56,23 +54,23 @@
 
 Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=2')
 
-tr = Test.AddTestRun("Test two client hello hooks")
+tr=Test.AddTestRun("Test two client hello hooks")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.StillRunningAfter = ts
-tr.StillRunningAfter = server
-tr.Processes.Default.Command = 'curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
-tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = "gold/preaccept-1.gold"
+tr.StillRunningAfter=ts
+tr.StillRunningAfter=server
+tr.Processes.Default.Command='curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode=0
+tr.Processes.Default.Streams.stdout="gold/preaccept-1.gold"

Review comment:
       Interesting. Is this spacing is accepted by pep 8? Normally for variable instantiations like this it prefers spaces around the `=`.




-- 
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] bneradt commented on a change in pull request #7639: Remove reference to client.verify.server from tests and other bits

Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7639:
URL: https://github.com/apache/trafficserver/pull/7639#discussion_r603406831



##########
File path: tests/gold_tests/tls_hooks/tls_hooks18.test.py
##########
@@ -56,23 +54,23 @@
 
 Test.PrepareTestPlugin(os.path.join(Test.Variables.AtsTestPluginsDir, 'ssl_hook_test.so'), ts, '-client_hello=2')
 
-tr = Test.AddTestRun("Test two client hello hooks")
+tr=Test.AddTestRun("Test two client hello hooks")
 tr.Processes.Default.StartBefore(server)
 tr.Processes.Default.StartBefore(Test.Processes.ts)
-tr.StillRunningAfter = ts
-tr.StillRunningAfter = server
-tr.Processes.Default.Command = 'curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
-tr.Processes.Default.ReturnCode = 0
-tr.Processes.Default.Streams.stdout = "gold/preaccept-1.gold"
+tr.StillRunningAfter=ts
+tr.StillRunningAfter=server
+tr.Processes.Default.Command='curl -k -H \'host:example.com:{0}\' https://127.0.0.1:{0}'.format(ts.Variables.ssl_port)
+tr.Processes.Default.ReturnCode=0
+tr.Processes.Default.Streams.stdout="gold/preaccept-1.gold"

Review comment:
       Oh, I think I see what's happening. If you check out line 45, I think that has a mistake in the patch that is throwing off the parser:
   
   https://github.com/apache/trafficserver/blob/7504f6726fed96e1b11b5bad1348af4932d1c202/tests/gold_tests/tls_hooks/tls_hooks18.test.py#L45
   
   I'll create a patch.




-- 
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