You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2021/11/15 16:04:47 UTC
[GitHub] [qpid-dispatch] ganeshmurthy opened a new pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
ganeshmurthy opened a new pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437
…ceiver and AsyncTestSender
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy closed pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
ganeshmurthy closed pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750616680
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1895,6 +1895,10 @@ def test_51_link_route_proxy_configured(self):
if self.skip['test_51'] :
self.skipTest("Test skipped during development.")
+ # The previous test waited for address CfgLinkRoute1 on router INT_B
+ # We will wait for address CfgLinkRoute1 to be unsubscribed
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
I agree with @jiridanek that the proper fix would to require each test to wait until all addresses introduced by the test have been purged from all the routers. But only having one test do this which happens to be ordered before the failing test is brittle in that there's nothing stopping some poor unfortunate dev from re-naming tests and inadvertently re-breaking this particular 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.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter commented on pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#issuecomment-969103629
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1437](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (44d5174) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/138d084d7a0d9dbeed2991affffc8f669563be68?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (138d084) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1437 +/- ##
==========================================
+ Coverage 84.71% 84.73% +0.02%
==========================================
Files 116 116
Lines 28617 28618 +1
==========================================
+ Hits 24242 24250 +8
+ Misses 4375 4368 -7
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...c/router\_core/modules/test\_hooks/core\_test\_hooks.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvdGVzdF9ob29rcy9jb3JlX3Rlc3RfaG9va3MuYw==) | `92.03% <0.00%> (-1.28%)` | :arrow_down: |
| [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3BhcnNlLmM=) | `87.96% <0.00%> (-0.22%)` | :arrow_down: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `93.73% <0.00%> (-0.22%)` | :arrow_down: |
| [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.35% <0.00%> (-0.06%)` | :arrow_down: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `90.11% <0.00%> (+0.09%)` | :arrow_up: |
| [src/iterator.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2l0ZXJhdG9yLmM=) | `89.47% <0.00%> (+0.18%)` | :arrow_up: |
| [src/adaptors/tcp\_adaptor.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL3RjcF9hZGFwdG9yLmM=) | `77.53% <0.00%> (+0.21%)` | :arrow_up: |
| [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `85.14% <0.00%> (+0.99%)` | :arrow_up: |
| [src/router\_core/agent\_conn\_link\_route.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2Nvbm5fbGlua19yb3V0ZS5j) | `85.38% <0.00%> (+1.16%)` | :arrow_up: |
| [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `82.84% <0.00%> (+4.14%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [138d084...44d5174](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#issuecomment-969103629
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1437](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (475f06e) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/138d084d7a0d9dbeed2991affffc8f669563be68?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (138d084) will **decrease** coverage by `0.03%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1437 +/- ##
==========================================
- Coverage 84.71% 84.67% -0.04%
==========================================
Files 116 116
Lines 28617 28617
==========================================
- Hits 24242 24232 -10
- Misses 4375 4385 +10
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `89.14% <0.00%> (-0.88%)` | :arrow_down: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `93.30% <0.00%> (-0.65%)` | :arrow_down: |
| [src/adaptors/http1/http1\_server.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX3NlcnZlci5j) | `85.18% <0.00%> (-0.56%)` | :arrow_down: |
| [src/parse.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3BhcnNlLmM=) | `87.96% <0.00%> (-0.22%)` | :arrow_down: |
| [src/adaptors/http1/http1\_codec.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL2FkYXB0b3JzL2h0dHAxL2h0dHAxX2NvZGVjLmM=) | `85.62% <0.00%> (-0.13%)` | :arrow_down: |
| [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21lc3NhZ2UuYw==) | `87.35% <0.00%> (-0.06%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.55% <0.00%> (+0.09%)` | :arrow_up: |
| [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `85.14% <0.00%> (+0.99%)` | :arrow_up: |
| [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `82.84% <0.00%> (+4.14%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [138d084...475f06e](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750093572
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1895,6 +1895,10 @@ def test_51_link_route_proxy_configured(self):
if self.skip['test_51'] :
self.skipTest("Test skipped during development.")
+ # The previous test waited for address CfgLinkRoute1 on router INT_B
+ # We will wait for address CfgLinkRoute1 to be unsubscribed
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
Depending on the specific order in which the tests are run sounds dangerous. I think that this line should be part of the previous test. It is probably sufficient to put it at the end. That way it would be skipped if the test failed before it got to it, but that should be rare,...
There is currently not a good way to have a per-test tear-down in system-tests. One could be developed... Maybe a decorator on the test method? Or go all-in with pytest and use fixtures, which do have their teardowns? The least disruptive thing would be to have a list in the test class, `self.addresses_requiring_unsubscribe`, which would be cleaned in the shared tear_down code. Tests would populate it with addresses that require this treatment.
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] ganeshmurthy commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
ganeshmurthy commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750645424
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1887,6 +1887,8 @@ def test_50_link_topology(self):
bc_a.close()
bc_b.close()
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
For now, I am going to commit this fix to see what is happening on the main branch. If we see more of these kinds of failures, let's then think of an approach to solving the issue in a more generic fashion
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r749506131
##########
File path: tests/system_test.py
##########
@@ -926,15 +926,15 @@ def get_queue_stats(self):
return self.queue_stats % (self.num_queue_puts, self.num_queue_gets)
def _main(self):
- self._container.timeout = 0.5
+ self._container.timeout = 3.0
Review comment:
Maybe use a global constant for the three seconds? So that the timeouts can be unified across tests, based on what they "mean". Something like `DEFAULT_TEST_CLIENT_CONTAINER_TIMEOUT = 3`?
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750624452
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1895,6 +1895,10 @@ def test_51_link_route_proxy_configured(self):
if self.skip['test_51'] :
self.skipTest("Test skipped during development.")
+ # The previous test waited for address CfgLinkRoute1 on router INT_B
+ # We will wait for address CfgLinkRoute1 to be unsubscribed
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
Another way, maybe not quick, but certainly dirty, would be to require that each test suffixes all addresses it uses with it's own name. The name can be found by introspecting the Python stack, or maybe the unittest module already knows the name of the currently running test. Randomly generated string would work as well.
The problem is that these unique addresses cannot be preconfigured ar the beginning, each test has to create them through management.
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r749508814
##########
File path: tests/system_test.py
##########
@@ -926,15 +926,15 @@ def get_queue_stats(self):
return self.queue_stats % (self.num_queue_puts, self.num_queue_gets)
def _main(self):
- self._container.timeout = 0.5
+ self._container.timeout = 3.0
Review comment:
In addition to system_tests.TIMEOUT which is there already, https://github.com/apache/qpid-dispatch/blob/138d084d7a0d9dbeed2991affffc8f669563be68/tests/system_test.py#L124-L126
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] jiridanek commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
jiridanek commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750093572
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1895,6 +1895,10 @@ def test_51_link_route_proxy_configured(self):
if self.skip['test_51'] :
self.skipTest("Test skipped during development.")
+ # The previous test waited for address CfgLinkRoute1 on router INT_B
+ # We will wait for address CfgLinkRoute1 to be unsubscribed
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
Depending on the specific order in which the tests are run sounds dangerous. I think that this line should be part of the previous test.
It is probably sufficient to put it at the end. That way it would be skipped if the test failed before it got to it, but that should be rare,...
There is currently not a good way to have a per-test tear-down in system-tests. One could be developed... Maybe a decorator on the test method? Or go all-in with pytest and use fixtures, which do have their teardowns? The least disruptive thing would be to have a list in the test class, `self.addresses_requiring_unsubscribe`, which would be cleaned in the shared tear_down code. Tests would populate it with addresses that require this treatment.
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] codecov-commenter edited a comment on pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#issuecomment-969103629
# [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1437](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddf49f6) into [main](https://codecov.io/gh/apache/qpid-dispatch/commit/fc975f2738e64e80cfe9f8dc552000e9e438225a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fc975f2) will **increase** coverage by `0.05%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## main #1437 +/- ##
==========================================
+ Coverage 84.69% 84.75% +0.05%
==========================================
Files 116 116
Lines 28619 28619
==========================================
+ Hits 24239 24255 +16
+ Misses 4380 4364 -16
```
| [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...router\_core/modules/edge\_router/link\_route\_proxy.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvbGlua19yb3V0ZV9wcm94eS5j) | `78.69% <0.00%> (-4.15%)` | :arrow_down: |
| [src/router\_core/modules/edge\_router/edge\_mgmt.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL21vZHVsZXMvZWRnZV9yb3V0ZXIvZWRnZV9tZ210LmM=) | `84.15% <0.00%> (-1.00%)` | :arrow_down: |
| [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `93.55% <0.00%> (+0.09%)` | :arrow_up: |
| [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `94.81% <0.00%> (+0.64%)` | :arrow_up: |
| [src/router\_core/delivery.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2RlbGl2ZXJ5LmM=) | `93.90% <0.00%> (+0.73%)` | :arrow_up: |
| [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `87.04% <0.00%> (+0.94%)` | :arrow_up: |
| [src/router\_core/connections.c](https://codecov.io/gh/apache/qpid-dispatch/pull/1437/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL3JvdXRlcl9jb3JlL2Nvbm5lY3Rpb25zLmM=) | `90.11% <0.00%> (+0.95%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [fc975f2...ddf49f6](https://codecov.io/gh/apache/qpid-dispatch/pull/1437?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750619527
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1887,6 +1887,8 @@ def test_50_link_topology(self):
bc_a.close()
bc_b.close()
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
Update: Jiri's approach is much cleaner - require all tests to verify that any addresses it introduced are removed from the router at the end of the individual test. But it would need to be done to all applicable tests, which means... more work.
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] asfgit merged pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437
--
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-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org
[GitHub] [qpid-dispatch] kgiusti commented on a change in pull request #1437: DISPATCH-2266: Increase container timeout to 3 seconds on AsyncTestRe…
Posted by GitBox <gi...@apache.org>.
kgiusti commented on a change in pull request #1437:
URL: https://github.com/apache/qpid-dispatch/pull/1437#discussion_r750544421
##########
File path: tests/system_tests_edge_router.py
##########
@@ -1887,6 +1887,8 @@ def test_50_link_topology(self):
bc_a.close()
bc_b.close()
+ self.INT_B.wait_address_unsubscribed("CfgLinkRoute1")
Review comment:
It would be better to do this check in test_51_link_route_proxy_configured to keep it all in one place and prevent accidental breakage if - for whatever reason - tests need to be re-ordered. Eventually all tests that enable a link route(s) should perform that type of check at the start of the 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.
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org