[
https://issues.apache.org/jira/browse/PROTON-2308?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17463738#comment-17463738
]
ASF GitHub Bot commented on PROTON-2308:
----------------------------------------
gemmellr commented on pull request #346:
URL: https://github.com/apache/qpid-proton/pull/346#issuecomment-999480504
> Although @gemmellr is correct in his analysis of what is going on in a
real AMQP messaging interaction, this is probably not all that important as the
C++ binding (at least currently) has no logic at either sender or receiver to
do anything special for dynamic nodes - it just passes the info on to the
application which is expected to validate and act on that information.
So from the point of view of this test it is only important to check that
the properties get correctly carried in both directions.
I very much disagree. The test should verify functionality actually works
the way it needs to be used, and thus help ensure someone doesn't go breaking
it later. A test of 'dynamic node [properties]' usage that doesnt actually use
the handling/options for dynamic nodes in the required way is simply not a good
test. It means it has never been verified to fully work in the situation it
needs to, and its never being fully verified it isnt broken later. Assuming
things work a certain way is how so many bugs come up when it later turns out
it doesnt.
If there is a test of the 'no node properties' dynamic usage elsewhere then
that would at least mean the 2 bits were somewhat tested in isolation even if
the node-props bit was still not actually in full (which would still be poor
but not quite as much). I might expect that other testing to be in this same
test class, but it doesnt appear so. Is that bit actually tested right now
anywhere?
>Unless the tester itself generates the requested nodes the API is not going
to so there really isn't anything to validate in the test.
In a future change to the C++ binding it would definitely make sense to
validate that you can't set address and set dynamic if you are sender/target or
receiver/source. And also that if dynamic node properties are set the dynamic
must also be set for the node, but that isn't done by the binding (or proton-c)
currently so testing for it makes no sense. Producing realistic protocol
interactions makes some sense but only to make the test "more realistic".
Youd simply check the request, set an address value to pass back, mark it
dynamic, and then verify the receiver got that address and flag. That would in
fact verify that the client options to set dynamic work as expected, that the
client is able to read back the provided dynamic address as it needs to be able
to, and inspect the dynamic flag was set appropriately. Thats not 'nothing to
validate', and is not about just being 'more realistic', but about actually
testing what needs to work to use this functionality.
I didnt suggest adding negative tests thats you cant set the things in the
illegal way you can currently, only that the test should cover testing it
actually does the things it should do when used the way it should be, which
appears untested.
--
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
> [cpp] Add support for setting Dynamic Node Properties
> -----------------------------------------------------
>
> Key: PROTON-2308
> URL: https://issues.apache.org/jira/browse/PROTON-2308
> Project: Qpid Proton
> Issue Type: Improvement
> Components: cpp-binding
> Affects Versions: proton-c-0.33.0
> Reporter: James Henry
> Assignee: Rakhi Kumari
> Priority: Major
> Labels: api-addition
>
> Requesting support for setting the dynamic node properties be added to the
> source and target options.
> This would allow the setting of termini node properties for senders and
> receivers.
> Similar to the following request made for Python here:
> https://issues.apache.org/jira/browse/PROTON-816
--
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
