Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hello @emilianbold ,
I have merged 310.
Can you close this one ?
Thank you
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/312
Hello,
Thanks for your PR, still can you answer my questions on PR 311 as there is
the same problem here.
Thank you
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/311
Thank you for your PR, still it seems there are more changes than described.
What is the real PR content ?
Only javadocs of CSVRead ?
Thank you
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Thanks @emilianbold , unfortunately I am still not able to generate diff
nor patch file to merge it.
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/306
Thanks @emilianbold for review.
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hi @emilianbold ,
AS you dropped your repository, I am not able to get the patch file to
merge it.
Any chance you have it ?
Thanks
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/306
This PR was merged through svn commit
http://svn.apache.org/viewvc?rev=1807719=rev
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/306
Hello
Unless there is a NO GO , I'll commit this tomorrow.
Thanks
---
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/304
Thanks for your PR.
Sorry for time taken to merge it, but as you know we are in summer period
and on holidays for some of us.
Feel free to test nightly build and give your feedback
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/302
Thank you for your contribution, there are some issues I indicated, either
fix them or I'll do when I have time ( not before 2 weeks) .
I don't see the code that plugs this sender.
Thankw
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/300#discussion_r130241103
--- Diff: src/core/org/apache/jmeter/gui/UndoHistory.java ---
@@ -160,63 +133,46 @@ public void add(JMeterTreeModel treeModel, String
comment
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hi Emilian,
Thanks for feedback.
For now with my last tests:
- I still have issues with check dirty when I do an intermediate save and
undo all changes, dirty is not detected
- I
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/299
Hello @emilianbold ,
I think we should wait for 3.3 release , don't you think so ?
Regards
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
It seems the creation order is important to reproduce:
First create Thread Group 1, add to it a Debug Sampler1
Second create Thread Group 2, add to it a Debug Sampler2
Add User Defined
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
I'm using code from:
- https://patch-diff.githubusercontent.com/raw/apache/jmeter/pull/300.patch
I have just found the problem.
I start jmeter using :
- t LAST
which
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Create a Tree like this:
Test Plan
|-User Defined Variable
|-Thread Group 1
|--TC1
|-Thread Group 2
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hi @emilianbold ,
The bug that you previously fixed with TreeState has reappeared.
Regards
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
HI @emilianbold ,
It seems to work correctly as of my tests for now, but it requires further
tests.
Testing it, I have noticed we have an existing bug with dirty check, it
seems
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/301
Hi @FSchumacher ,
Thanks for the PR.
Maybe we could add a interface that plugins could implement to register
additional classes:
Too coarse and risky
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/301#discussion_r128925907
--- Diff: src/core/org/apache/jmeter/util/JMeterUtils.java ---
@@ -1261,10 +1265,39 @@ public static final void refreshUI() {
*/
public
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Maybe we could change how dirty check works which I think was here because
there was no Redo/Undo Management.
Am I wrong to think dirty may be simpler to compute now ?
- Any change
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
@emilianbold , do you think computing dirty should be part of the undoable
edit ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hi Emilian,
Thanks for the fix for last bug.
I cannot provide the 900K test plan as it's a real life one from a
customer. I'll try to create a fake one.
Anyway, I think the most
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hello Emilian,
1) Thanks for fix, see my comment
2) I've noticed another issue.
Create a Tree like this:
Test Plan
|-User Defined Variable
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/300#discussion_r128917971
--- Diff: src/core/org/apache/jmeter/gui/action/ActionRouter.java ---
@@ -67,7 +67,10 @@ public void actionPerformed(final ActionEvent e
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hi @undera,
I agree, but for now it is not yet fully fixed although a lot of progress
have been made thanks to Emilian.
Regards
---
If your project is set up for it, you can reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/300
Hello Emilian,
Thanks for this great piece of work !
First test look very promising in regards of performances.
I have a first issue to report:
1. Open JMeter
2. Select
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/298
Hi Felix,
I suggest you commit the PR so that we can update and test code more easily.
Thanks
Regards
---
If your project is set up for it, you can reply to this email and have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/298
Hi Felix,
Looks good to me.
I'd just change a bit in HTTPJavaImpl#getHeaders(HeaderManager
headerManager) to:
`private Header[] getHeaders(HeaderManager headerManager
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/298
Great @FSchumacher !
Thanks for your work !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/298
Thanks Felix for PR.
I reviewed it, it looks good to me.
Maybe since we're making API modifications we should start thinking about:
- https://bz.apache.org/bugzilla/show_bug.cgi?id=53540
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/261
Hello @squarebracket ,
Sorry for late reply.
Thinking about it rapidly, I think your analysis is good if we were writing
a new API.
But I am afraid some code in JMeter relies
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/295
Hi Team,
What shall we do ?
As per Felix note on dev mailing list, it is more an algorithm variation
than a bug.
---
If your project is set up for it, you can reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/296
Hi Team,
What shall we do ?
As per Felix note on dev mailing list, it is more an algorithm variation
than a bug.
---
If your project is set up for it, you can reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/292
Hi Mabye a SyntaxtTextArea for result would be better
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/292
Hello,
It might be interesting to add some Text area to input some JSR223 Code to
fill-in the context.
As a fist step, we should document this and possibly add a Hint that pops
up
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/296
Hello @abalanonline ,
Thanks for your replies and explanations !
I am not a math expert as you seem to be, so I have few questions you may
be able to help on:
1. Thanks to your
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/296
Thanks for your patch.
This test fails for me:
`@Test
public void testPercentagePointBug() throws Exception {
long values[] = new long[] {
10L,9L
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/291
Hello,
Regarding format, for maintainability and documentation, using Java8
Duration is better IMO.
Regards
---
If your project is set up for it, you can reply to this email and have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/295
Hello @abalanonline ,
Thanks for your PR.
Would it be possible to provide a current failing test case that shows the
issue and confirms that your fix is ok ?
Thank you
---
If your
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/294
Hi,
I think the greatest UI would contain a Search field where you could enter
a text of XPATh and the Tree would expand all nodes matching the search and
collapsing others.
Regards
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/294
Looks useful to me also although I think an XML formatter which allows
collapsing of nodes in editor would be much better.
---
If your project is set up for it, you can reply to this email
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/293
+1 for fixing both methods
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/293
@FSchumacher , I agree. Let's commit the patch without the new method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/291
Thanks for the changes. Looks good to me for commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/292
Hi @max3163 ,
Interesting idea.
Does your implementation handle all functions ?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/293
Thanks for analysis and patch. It was a hard one !.
@FSchumacher , I don't understand your comment , what exactly do you want
to change ?
Thanks
---
If your project is set up
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/291
Hello,
Looks good to me (see my comments), very good idea !
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113294213
--- Diff:
src/functions/org/apache/jmeter/functions/TimeShiftingFunction.java ---
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113294131
--- Diff:
src/functions/org/apache/jmeter/functions/TimeShiftingFunction.java ---
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113294069
--- Diff:
src/functions/org/apache/jmeter/functions/TimeShiftingFunction.java ---
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113294026
--- Diff:
src/functions/org/apache/jmeter/functions/TimeShiftingFunction.java ---
@@ -0,0 +1,186 @@
+/*
+ * Licensed to the Apache Software
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113292969
--- Diff: src/core/org/apache/jmeter/resources/messages_fr.properties ---
@@ -1169,6 +1170,7 @@ throughput_control_perthread_label=Par utilisateur
Github user pmouawad commented on a diff in the pull request:
https://github.com/apache/jmeter/pull/291#discussion_r113292880
--- Diff: src/core/org/apache/jmeter/resources/messages_fr.properties ---
@@ -238,6 +238,7 @@ database_sql_query_title=Requ\u00EAte SQL JDBC par
d
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/288
Thanks for contribution.
LGTM, I agree with Felix.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/283
Hello Maxime,
Thanks for patch.
My remarks:
- I think we need here JUnit tests to check no regression will occur. This
part of the code is complex and critical and needs 100% coverage
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/279
Hello,
Looks good to me.
Regards
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/277
Hello,
Thank you for your contribution.
My notes:
- The class ExtendedQueueRequestor seems to come from existing code as per
"This source code implements specifications de
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/247
Merged with commit:
http://svn.apache.org/viewvc?rev=1783297=rev
This PR can be closed.
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/241
Hello,
Would it be possible to bench this with more threads than 2 ?
50 for example ?
Would it be possible to wait for 3.2 release before merging this one ?
Thanks
---
If your
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/247
Hi @max3163 ,
As you're a commiter now, could you apply it ?
I tried to but it seems the generated patch files contains other files
(JSONManager) , I don't know why but there seem
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/263
Hi Woonsan,
Thanks for the PR.
I just modified few lines, I think I did right, but confirmation is better.
I added log.isDebugEnabled() when parameters of log message called some
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/254
I'm not sure commons-logging can be dropped as it's a compile dependency of
HttpClient 4.5.3:
https://repo1.maven.org/maven2/org/apache/httpcomponents/httpclient/4.5.3/httpclient-4.5.3.pom
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/260
Hello,
Thanks for contribution.
I am not sure we should apply this fix as I am not sure initial bug report
was correct.
AFAIU, the propMap is never accessed concurrently although I
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/257
Hello,
Another question I have.
I see you replace StringBuilder by '+' concat.
Is there a reason for this ?
Reading:
-
http://www.pellegrino.link/2015/08/22/string-concatenation
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/247
Hello,
I reviewed this PR and I have some blocker remarks so I didn't merge it:
- I think code in StandardJMeterEngine and DistributedRunner is duplicated
- I wouldn't put any dependency
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/257
@ham1 , Thanks a lot for your contribution.
Looks globally good for me.
I have just one remark, I wouldn't "optimize" the org.apache.jmeter.control
package classes.
This part
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/253
@max3163 ,
Ok for your proposal.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/253
@max3163 , I've fixed the missing httpcore-nio.
For the bug, it's because you schedule every 5s, so test ends before and
closes the Sender and just guarantees end is sent.
We could fix
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/253
Hello
I want something similar to that .
In next evolution, I think it would be useful to add tags as a parameter of
the InfluxdbBackendListenerClient.
For example, users could set
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/253
Hello @max3163 ,
I merged this PR with following fixes/changes:
- Added more configuration on Async Http Client (Reactor)
- Fixed that End annotation not being sent
- Fixed the label
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/253
Hello Maxime,
Thanks for being so active on JMeter these days ! I'm really happy about it.
I reviewed PR, it looks globally good except for something I don't
understand.
Why do you
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/246
Hi Mark,
Good to know.
Are you using Apache HttpAsyncClient or something else ?
Do you agree with my proposals ?
Thanks
---
If your project is set up for it, you can reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/246
Hello,
Thanks for contribution.
I have few remarks on implementation:
- You intentionally don't set timestamp. I am not sure it's a good idea as
if any delay occurs sending to InfluxDB
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/240
Hello,
Thanks for PR.
I have commited it but with some slight changes.
Could you test nightly build and give us feedback ?
Also could you attach the JMX plan that you attached
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/250
Thanks for PR.
It looks ok.
Before we start integrating it, would it be possible to propose a PR for
the configuration part ?
I think it would be interesting to see how configuration
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/245
Hello,
I have commited part of the PR and made some changes:
- I didn't want to use everywhere HeaderAsPropertyRenderer.install as it
removes formatting without introducing sorting
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/249
Thanks for contribution but it seems you've based it on a very old version
of code (2.5.1) so it's very hard to see what you changed.
Can you use last trunk and submit a new pr
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/248
Hello,
Thanks for your contribution.
But are you sure this issue was not already fixed within:
https://bz.apache.org/bugzilla/show_bug.cgi?id=60049
By commit:
http
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/237
Hello,
Thanks for proposal.
What is the quantity of code ? Does it have any dependency on Influx
library or it is standalone using socket ?
Thank you
---
If your project is set up
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/237
@max3163 , are you using GraphiteBackendListener or something else (I don't
fully understand your comment on "I'm thinking to send a PR to share it too".
If not, why aren't
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/238
Thanks for contribution.
I reviewed the patch. It doesn't break current code but I think there may
be an issue in :
`Map.Entry<JMeterThread, Thread> entry =
allThreads.en
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/237
Hello Team,
PR looks fine to me.
Do you agree with enhancement ?
Thank you
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/240
Thanks for contribution.
Few notes:
- docs (component-reference.xml) should be updated
- In handleError, I think you should surround close line 232 with try/catch
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/241
Hello @loganmzz ,
Is this PR a duplicate of PR 239 ?
If so, could you close 239?
Did you check the performance impact of this feature ?
Thanks a lot for your contribution
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/227
Hi Team,
Shall I merge this one ?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/237
Hello @max3163,
Thank you for your contribution.
Could you (as requested by Felix) give more details on the use case for
this PR ?
Thank you
Regards
---
If your project is set up
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/236
Thanks a lot for your screenshots which give a better view of the feature
and your work on this.
Unfortunately As discussed in :
- https://bz.apache.org/bugzilla/show_bug.cgi?id=59937
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/236
Hello,
Thanks for contribution.
Could you provide a screenshot of what the result will look like ?
Thank you
---
If your project is set up for it, you can reply to this email and have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi @blindio ,
Did you have time working on the reported issues ?
Thanks
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi Again,
For info, in your commit, no need to include the HTML files, only XML for
documentation.
Thanks
---
If your project is set up for it, you can reply to this email and have your
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Hi Thorson,
Sorry for late reply, we are in the middle of 3.1 release preparation and I
was on holiday.
Thanks again for you PR.
I started reviewing, few notes
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/233
Thanks for this contribution !
We will review it ASAP and give you feedback.
Regards
Philippe M.
---
If your project is set up for it, you can reply to this email and have your
reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/231
Hello,
Thanks for the patch.
First review look good to me.
Any reason for not using ThreadLocalRandom.current() instead of Random ?
Regards
Philippe
---
If your project
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/230
+1 for me
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/221
Hello,
I'll wait 2 more days for reaction on guava subject.
Unless there is a nogo, I'll commit the patch as is.
Regards
---
If your project is set up for it, you can reply
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/223
Hello,
In my opinion, you should commit it and we can improve it with further
commits.
I agree with @vlsi on the performance involvement of warn logging , but
we'll find a solution if you
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/225
Hello,
I find it interesting to have the version of libraries .
So I am not keen on changing it.
Regards
---
If your project is set up for it, you can reply to this email and have
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/224
+1 for commiting it
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so
Github user pmouawad commented on the issue:
https://github.com/apache/jmeter/pull/224
Yes you're correct, I meant Text renderer but in that case formatting does
not really make sense as it can be anything (not only html)
---
If your project is set up for it, you can reply
101 - 200 of 267 matches
Mail list logo