[
https://issues.apache.org/jira/browse/DISPATCH-2066?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17491297#comment-17491297
]
ASF GitHub Bot commented on DISPATCH-2066:
------------------------------------------
jiridanek commented on a change in pull request #1516:
URL: https://github.com/apache/qpid-dispatch/pull/1516#discussion_r805142665
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -800,15 +799,37 @@ def _read_until_empty(self, sock, timeout):
break # timeout
return data
+ def check_logs(self, prefix, log_file):
+ # check router log for proper block/unblock activity
+ block_ct = 0
+ unblock_ct = 0
+ block_line = 0
+ unblock_line = 0
+ line_no = 0
+ with io.open(log_file) as f:
Review comment:
```suggestion
with open(log_file) as f:
```
That is just alias for the built in `open`, using it through the module name
is IMO unusual. And `io.open == open` evaluates to True
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -800,15 +799,37 @@ def _read_until_empty(self, sock, timeout):
break # timeout
return data
+ def check_logs(self, prefix, log_file):
+ # check router log for proper block/unblock activity
Review comment:
```suggestion
"""Check router log for proper block/unblock activity"""
```
Makes it render in a pleasing green, instead of indifferent gray for me (in
PyCharm).
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -905,7 +918,7 @@ def test_02_backpressure_server(self):
client_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Review comment:
The leaking sockets probably don't matter too much in this test. A
leaking router would be a bigger problem.
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -951,18 +964,8 @@ def test_02_backpressure_server(self):
server_listener.shutdown(socket.SHUT_RDWR)
server_listener.close()
- # search the router log file to verify Q2 was hit
-
- block_ct = 0
- unblock_ct = 0
- with io.open(self.INT_A.logfile_path) as f:
- for line in f:
- if 'server link blocked on Q2 limit' in line:
- block_ct += 1
- if 'server link unblocked from Q2 limit' in line:
- unblock_ct += 1
- self.assertTrue(block_ct > 0)
- self.assertEqual(block_ct, unblock_ct)
+ router.teardown()
+ self.check_logs("server", router.logfile_path)
Review comment:
Since there is one server started and stopped in every test method, I'd
put the creation into `def setUp` and teardown into `tearDown` methods.
e.g.
```python
def tearDown(self)
router.teardown()
self.check_logs("server", router.logfile_path)
super().tearDown()
```
The way it is now, if test fails, router is not stopped.
If we ever decided to switch to exclusively to pytest, then qdrouterd could
be managed as pytest fixture
(https://docs.pytest.org/en/7.0.x/explanation/fixtures.html).
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -800,15 +799,37 @@ def _read_until_empty(self, sock, timeout):
break # timeout
return data
+ def check_logs(self, prefix, log_file):
+ # check router log for proper block/unblock activity
+ block_ct = 0
+ unblock_ct = 0
+ block_line = 0
+ unblock_line = 0
+ line_no = 0
+ with io.open(log_file) as f:
+ for line in f:
Review comment:
```suggestion
for line_no, line in enumerate(f, start=1):
```
##########
File path: tests/system_tests_http1_adaptor.py
##########
@@ -905,7 +918,7 @@ def test_02_backpressure_server(self):
client_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
Review comment:
```suggestion
with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as client_sock
```
or do the `try/finally` around it, to ensure that socket does not leak if
the test fails before socket is closed.
Context manager (`with`) is IMO preferrable, when it can be used. Here you
have three sockets, so it does not work all that well (requires three levels of
nesting the `with`s). I'd go with `with` here, after all.
(I realize this is old code, so I understand if you don't want to change it
more than necessary.)
--
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: dev-unsubscr...@qpid.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> [http1] In system_tests_http1_adaptor Http1AdaptorQ2Standalone backpressure
> tests, block_ct != unblock_ct
> ---------------------------------------------------------------------------------------------------------
>
> Key: DISPATCH-2066
> URL: https://issues.apache.org/jira/browse/DISPATCH-2066
> Project: Qpid Dispatch
> Issue Type: Bug
> Components: Protocol Adaptors
> Affects Versions: 1.16.0
> Reporter: Jiri Daněk
> Assignee: Ken Giusti
> Priority: Major
> Labels: HTTP/1.x
> Fix For: 1.19.0
>
>
> https://travis-ci.com/github/apache/qpid-dispatch/jobs/499996271#L11501
> {noformat}
> test 70
> Start 70: system_tests_http1_adaptor
> 70: Test command: /usr/bin/python3.8
> "/home/travis/build/apache/qpid-dispatch/build/tests/run.py" "-m" "unittest"
> "-v" "system_tests_http1_adaptor"
> 70: Test timeout computed to be: 500
> 70: test_01_unsolicited_response
> (system_tests_http1_adaptor.Http1AdaptorBadEndpointsTest)
> 70: Create a server that sends an immediate Request Timeout response ... ok
> 70: test_02_bad_request_message
> (system_tests_http1_adaptor.Http1AdaptorBadEndpointsTest)
> 70: Test various improperly constructed request messages ... ok
> 70: test_03_bad_response_message
> (system_tests_http1_adaptor.Http1AdaptorBadEndpointsTest)
> 70: Test various improperly constructed response messages ... ok
> 70:
> 70: Router TestBadEndpoints output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 1 24 ^pn_condition$
> 70: 1 1536 ^pn_raw_connection$
> 70: 1 56 qdr_core_subscribe
> 70: 2 112 ^pn_string_grow$
> 70: 7 328 ^pn_object_new$
> 70: 1 128 ^pn_list$
> 70: 2 48 ^pni_record_create$
> 70: 597 926857 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70: test_01_concurrent_requests
> (system_tests_http1_adaptor.Http1AdaptorEdge2EdgeTest)
> 70: Test multiple concurrent clients sending streaming messages ... ok
> 70: test_02_credit_replenish
> (system_tests_http1_adaptor.Http1AdaptorEdge2EdgeTest)
> 70: Verify credit is replenished by sending > the default credit window ... ok
> 70: test_03_server_reconnect
> (system_tests_http1_adaptor.Http1AdaptorEdge2EdgeTest)
> 70: Verify server reconnect logic. ... ok
> 70: test_04_server_pining_for_the_fjords
> (system_tests_http1_adaptor.Http1AdaptorEdge2EdgeTest)
> 70: Test permanent loss of server ... ok
> 70: test_05_large_streaming_msg
> (system_tests_http1_adaptor.Http1AdaptorEdge2EdgeTest)
> 70: Verify large streaming message transfer ... ok
> 70:
> 70: Router EA2 output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 7 59 ^load_server_config$
> 70: 6 401 ^qd_dispatch_configure_connector$
> 70: 1 24 ^pn_condition$
> 70: 1 1536 ^pn_raw_connection$
> 70: 1 64 sys_mutex
> 70: 1 56 qdr_core_subscribe
> 70: 2 112 ^pn_string_grow$
> 70: 7 328 ^pn_object_new$
> 70: 1 128 ^pn_list$
> 70: 2 48 ^pni_record_create$
> 70: 601 929697 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70:
> 70: Router EA2 debug dump file:
> 70: >>>>
> 70: alloc.c: Items of type 'qd_timer_t' remain allocated at shutdown: 1
> (SUPPRESSED)
> 70: alloc.c: Items of type 'qd_connector_t' remain allocated at shutdown: 1
> (SUPPRESSED)
> 70:
> 70: <<<<
> 70:
> 70: Router EA1 output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 7 59 ^load_server_config$
> 70: 6 401 ^qd_dispatch_configure_connector$
> 70: 1 64 sys_mutex
> 70: 1 56 qdr_core_subscribe
> 70: 601 929697 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70:
> 70: Router EA1 debug dump file:
> 70: >>>>
> 70: alloc.c: Items of type 'qd_timer_t' remain allocated at shutdown: 1
> (SUPPRESSED)
> 70: alloc.c: Items of type 'qd_connector_t' remain allocated at shutdown: 1
> (SUPPRESSED)
> 70:
> 70: <<<<
> 70:
> 70: Router INT.A output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 6 336 qdr_core_subscribe
> 70: 633 956953 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70: test_01_mgmt (system_tests_http1_adaptor.Http1AdaptorManagementTest)
> 70: Create and delete HTTP1 connectors and listeners ... ok
> 70:
> 70: Router HTTP1MgmtTest output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 1 56 qdr_core_subscribe
> 70: 637 951241 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70: test_000_stats (system_tests_http1_adaptor.Http1AdaptorOneRouterTest) ...
> ok
> 70: test_001_get (system_tests_http1_adaptor.Http1AdaptorOneRouterTest) ... ok
> 70: test_002_head (system_tests_http1_adaptor.Http1AdaptorOneRouterTest) ...
> ok
> 70: test_003_post (system_tests_http1_adaptor.Http1AdaptorOneRouterTest) ...
> ok
> 70: test_004_put (system_tests_http1_adaptor.Http1AdaptorOneRouterTest) ... ok
> 70: test_005_get_10 (system_tests_http1_adaptor.Http1AdaptorOneRouterTest)
> ... ok
> 70: test_006_head_10 (system_tests_http1_adaptor.Http1AdaptorOneRouterTest)
> ... ok
> 70: test_007_post_10 (system_tests_http1_adaptor.Http1AdaptorOneRouterTest)
> ... ok
> 70: test_008_put_10 (system_tests_http1_adaptor.Http1AdaptorOneRouterTest)
> ... ok
> 70:
> 70: Router INT.A output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 2 48 ^pn_condition$
> 70: 2 3072 ^pn_raw_connection$
> 70: 1 56 qdr_core_subscribe
> 70: 10 496 ^pn_object_new$
> 70: 2 256 ^pn_list$
> 70: 4 96 ^pni_record_create$
> 70: 598 927433 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70:
> 70: Router INT.A debug dump file:
> 70: >>>>
> 70: alloc.c: Items of type 'qd_buffer_t' remain allocated at shutdown: 32
> (SUPPRESSED)
> 70:
> 70: <<<<
> 70: test_01_backpressure_client
> (system_tests_http1_adaptor.Http1AdaptorQ2Standalone)
> 70: Trigger Q2 backpressure against the HTTP client. ... FAIL
> 70: test_02_backpressure_server
> (system_tests_http1_adaptor.Http1AdaptorQ2Standalone)
> 70: Trigger Q2 backpressure against the HTTP server. ... FAIL
> 70:
> 70: Router RowdyRoddyRouter output file:
> 70: >>>>
> 70: -----------------------------------------------------
> 70: Suppressions used:
> 70: count bytes template
> 70: 1 56 qdr_core_subscribe
> 70: 595 925705 /libpython3.*.so
> 70: -----------------------------------------------------
> 70:
> 70:
> 70: <<<<
> 70:
> 70: ======================================================================
> 70: FAIL: test_01_backpressure_client
> (system_tests_http1_adaptor.Http1AdaptorQ2Standalone)
> 70: Trigger Q2 backpressure against the HTTP client.
> 70: ----------------------------------------------------------------------
> 70: Traceback (most recent call last):
> 70: File
> "/home/travis/build/apache/qpid-dispatch/tests/system_tests_http1_adaptor.py",
> line 740, in test_01_backpressure_client
> 70: self.assertEqual(block_ct, unblock_ct)
> 70: AssertionError: 26 != 24
> 70:
> 70: ======================================================================
> 70: FAIL: test_02_backpressure_server
> (system_tests_http1_adaptor.Http1AdaptorQ2Standalone)
> 70: Trigger Q2 backpressure against the HTTP server.
> 70: ----------------------------------------------------------------------
> 70: Traceback (most recent call last):
> 70: File
> "/home/travis/build/apache/qpid-dispatch/tests/system_tests_http1_adaptor.py",
> line 821, in test_02_backpressure_server
> 70: self.assertEqual(block_ct, unblock_ct)
> 70: AssertionError: 41 != 39
> 70:
> 70: ----------------------------------------------------------------------
> 70: Ran 20 tests in 128.976s
> 70:
> 70: FAILED (failures=2)
> 70/74 Test #70: system_tests_http1_adaptor ........................***Failed
> 129.29 sec
> {noformat}
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org
