[GitHub] jmeter issue #325: 61544 - Added read, browse and clear as communication sty...
Github user bennyvw commented on the issue:
https://github.com/apache/jmeter/pull/325
Philippe,
In my branch 61544, I did not have the JMS_TESTS files (jmx, xml, csv). I
don't know why, do you?
I added them manually from trunk, even after merging trunk into 61544 I did
not have them.
The issues that were introduced in those tests have been solved and I have
added tests for Read, Browse and Clear queue. Because Clear queue did not work
on the ActiveMQ queues, I changed the code slightly.
Just solved the merge conflicts in JMS_TESTS.JMX.
Could you please give it another review?
Regards,
Benny.
> Op 14 november 2017 om 18:24 schreef Philippe M
:
>
>
> Hello @bennyvw https://github.com/bennyvw ,
> Thanks for updating PR.
> It seems the changes have introduced an issue in JMS_TESTS.jmx which
is located in source files on github/svn (only, not in binary) in bin/testfiles:
>
https://github.com/apache/jmeter/blob/trunk/bin/testfiles/JMS_TESTS.jmx
>
> To debug the issue, put activemq-all.jar in lib folder and run the
test.
>
> Thanks
>
> â
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
https://github.com/apache/jmeter/pull/325#issuecomment-344333265 , or mute the
thread
https://github.com/notifications/unsubscribe-auth/Aem4XtE1GQcFri2ghrxZpyxu9skZZjiFks5s2czhgaJpZM4QZnSl
.
>
>
>
Met vriendelijke groet,
Benny van Wijngaarden.
Tel. 0629556587http://www.smaragd-it.nl/
be...@smaragd-it.nl mailto:be...@smaragd-it.nl
---
[GitHub] jmeter issue #325: 61544 - Added read, browse and clear as communication sty...
Github user bennyvw commented on the issue: https://github.com/apache/jmeter/pull/325 Processed your remarks, @pmouawad. Could you please review it again? And you might give us some hint why tests are failing in Travis CI. It complains about JMS_TESTS.JMX, but I did not find that file.. ---
[GitHub] jmeter issue #325: 61544 - Added read, browse and clear as communication sty...
Github user bennyvw commented on the issue:
https://github.com/apache/jmeter/pull/325
Philippe,
Thanks for your quick review. I am going to pay attention to your remarks
asap.
Regards,
Benny.
> Op 10 november 2017 om 21:20 schreef Philippe M
:
>
>
> @pmouawad requested changes on this pull request.
>
> Thanks a lot for this very interesting contribution.
> Find my remarks below.
> I'll be happy to merge this PR once you can take into account those
remarks and possible remarks from other members.
> Regards
>
>
> -
>
> In docs/usermanual/component_reference.html
https://github.com/apache/jmeter/pull/325#discussion_r150327021 :
>
> > @@ -3813,7 +3813,7 @@
>
>
> HTML files are generated from XML. Could you update
comonent_reference.xml instead ?
>
>
> -
>
> In docs/usermanual/component_reference.html
https://github.com/apache/jmeter/pull/325#discussion_r150327166 :
>
> > +queue referenced by message.getJMSReplyTo().
> +
> +
> +
> +Read
> +
> + will read a message from an outgoing queue which has no
listeners attached. This can be convenient for testing purposes.
> + This method can be used if you need to handle queues without a
binding file (in case the jmeter-jms-skip-jndi library is used),
> + which only works with the JMS Point-to-Point sampler.
> + In case binding files are used, one can also use the JMS
Subscriber Sampler for reading from a queue.
> +
> +
> +
> +Browse
> +
> + will determine the current queue depth without removing
messages from the queue, returning the number of messages on the queue.
>
> Would it be possible to provide screenshots as it makes documentation
much clearer
>
>
> -
>
> In
src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/JMSSampler.java
https://github.com/apache/jmeter/pull/325#discussion_r150327668 :
>
> > res.setResponseMessage(e.getLocalizedMessage());
> }
> }
> res.sampleEnd();
> return res;
> }
>
> +private void handleBrowse(SampleResult res) throws JMSException {
> +LOGGER.debug("isBrowseOnly");
> +StringBuffer sb = new StringBuffer("");
>
> StringBuilder would be better here, and wherever StringBuffer is used
>
>
> -
>
> In
src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/JMSSampler.java
https://github.com/apache/jmeter/pull/325#discussion_r150328109 :
>
> > +Integer.parseInt(getPriority()),
Long.parseLong(getExpiration()));
> +res.setRequestHeaders(Utils.messageProperties(msg));
> +if (replyMsg == null) {
> +res.setResponseMessage("No reply message received");
> +} else {
> +if (replyMsg instanceof TextMessage) {
> +res.setResponseData(((TextMessage)
replyMsg).getText(), null);
> +} else {
> +res.setResponseData(replyMsg.toString(), null);
> +}
> +
res.setResponseHeaders(Utils.messageProperties(replyMsg));
> +res.setResponseOK();
> +}
> +}
> +
> +private String browseQueueForConsumption(Queue queue, String
jmsSelector, SampleResult res, StringBuilder buffer,
>
> Isn't name confusing here ? AFAIU we are consuming here not browsing
right ?
>
>
> -
>
> In
src/protocol/jms/org/apache/jmeter/protocol/jms/sampler/JMSSampler.java
https://github.com/apache/jmeter/pull/325#discussion_r150328248 :
>
> > +} else {
> +res.setResponseData(replyMsg.toString(), null);
> +}
> +
res.setResponseHeaders(Utils.messageProperties(replyMsg));
> +res.setResponseOK();
> +}
> +}
> +
> +privat
[GitHub] jmeter pull request #325: 61544 - Added read, browse and clear as communicat...
GitHub user bennyvw opened a pull request: https://github.com/apache/jmeter/pull/325 61544 - Added read, browse and clear as communication styles to JMSSampler ## Description In JMS Point-to-Point Sampler, besides the standard options for communication style, Request Only and Request Response, we added options to Read a message, Clear the queue (purge all messages) and Browse the queue (count number of messages on the queue). The property JMSSampler.connectionmode is replaced by an JMSSampler.communicationstyle. This way, multiple languages for the options are supported. ## Motivation and Context It does not solve a problem, but adds some functionality. ## How Has This Been Tested? This change is NOT backwards compatible in that it will not find the intProp communicationStyle in existing scripts, thus resetting them to 'Clear' option. Hmm, typing this I realize that 'Request Only' might have been the better default option to save existing functionality where possible. ## Screenshots (if appropriate): ## Types of changes - [ ] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [X] Breaking change (fix or feature that would cause existing functionality to not work as expected) ## Checklist: - [X] My code follows the code style of this project. - [X] My change requires a change to the documentation. - [X] I have updated the documentation accordingly. You can merge this pull request into a Git repository by running: $ git pull https://github.com/bennyvw/jmeter 61544 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/jmeter/pull/325.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #325 commit 12a68e3777be4df9b588a2c47994ad059ffe1aa7 Author: DELL_WORK\manfred Date: 2017-09-20T09:33:28Z Added jms communication styles (read, browse and clear). commit 9e6cc05f0a96d8c34de0f53a5a6b78047637cd71 Author: DELL_WORK\manfred Date: 2017-10-31T11:23:20Z Updated documentation with Read, Browse, Clear actions for the JMSSampler. Renamed connectionMode to communicationStyle. commit 60c96bffc3dde7aeca669efa09fde6c2325a5486 Author: Manfred M. Schürhoff Date: 2017-11-07T17:52:04Z Fixed compilation issues (removed unused packages). commit 539b7adf33efba15d936122b2cb56c48997edfd5 Author: Benny van Wijngaarden Date: 2017-11-09T15:31:45Z Changed formatting for the sake of checkstyle ---
