You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jmeter.apache.org by "markuswege (via GitHub)" <gi...@apache.org> on 2023/05/10 06:03:36 UTC
[GitHub] [jmeter] markuswege opened a new pull request, #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
markuswege opened a new pull request, #5900:
URL: https://github.com/apache/jmeter/pull/5900
## Description
Bugfix: https://github.com/apache/jmeter/issues/5896
User Defined Variables are not evaluated on client (master) in distributed setups. This has beed fixed by a second pass through the test tree after it is sent to the servers.
## Motivation and Context
We are using a distributed setup for our tests. Credentials and some configurations are passed via properties into the client (master). It occurred that the BackendListener, using User Defined Variables, received unevaluated strings as credentials (see bugticket linked for details).
In order to prevent side effects for test party running on the servers, we decided a second pass over the needed testelements after sending the unmodified test to the servers would be best.
## How Has This Been Tested?
Manual testing using debug logging.
## Types of changes
- Bug fix (non-breaking change which fixes an issue)
## Checklist:
- [X] My code follows the [code style][style-guide] of this project.
- [ ] I have updated the documentation accordingly.
[style-guide]: https://wiki.apache.org/jmeter/CodeStyleGuidelines
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1541486503
Can you wrap it as batch test server test?
See https://github.com/apache/jmeter/blob/193bd552d492628e31b4c72bc2a5085ed0b14126/src/dist-check/build.gradle.kts#LL233C1-L233C1 , https://github.com/apache/jmeter/blob/193bd552d492628e31b4c72bc2a5085ed0b14126/bin/testfiles/BatchTestLocal.jmx
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1541409218
Do you think you could add a test case for the change?
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1541406192
Please keep `.idea/icon.png` intact.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1547450222
>So, why are there no tests for ClientJmeterEngine?
Try adding `if (true) { throw new IllegalStateException("ClientJmeterEngine is broken"); }` to `ClientJmeterEngine` in your PR.
Then you'll see what are the tests that can detect such a change.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] markuswege commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "markuswege (via GitHub)" <gi...@apache.org>.
markuswege commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1569640403
After i while i refuse to write a test for this.
Why?
Because there is simply not a single test for the core classes. Few parts are covered indeirectly by tests.
Nevertheless, as you say: Hard to maintain, also hard to write tests for a pretty simple fix without any other tests doing some basic stuff like creating the TestTree from scratch.
Please either check manually or trust in our team programming efforts to fix this.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1578361550
> hard to write tests for a pretty simple fix without any other tests doing some basic stuff like creating the TestTree from scratch.
Please check https://github.com/apache/jmeter/blob/5bb56691f93bb7c6b6bee03a2f9a66a562199f51/src/components/src/test/kotlin/org/apache/jmeter/threads/openmodel/OpenModelThreadGroupConfigElementTest.kt#L39-L85
It creates the test tree from scratch.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] markuswege commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "markuswege (via GitHub)" <gi...@apache.org>.
markuswege commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1541429460
[Stripped.zip](https://github.com/apache/jmeter/files/11438767/Stripped.zip)
Testfile attached. To be run in distributed mode with
`-JactiveProfileId=StrippedProfile -GactiveProfileId=StrippedProfile -Jpassword_influxdb_qat=PshYUe4Qsalv3G6Rywki3qT56CsLqcaN`
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] markuswege commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "markuswege (via GitHub)" <gi...@apache.org>.
markuswege commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1547425567
Undestandable.
So, why are there no tests for ClientJmeterEngine?
Test coverage run in IntelliJ over all included tests show no coverage of ClientJMeter Engine or the according if-branch in PreCompiler.
If there were Tests, i would have added Testscases, but i simply don't have the time to create Tests for ClientJMeterEngine and PreCompiler from scratch.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Bugfix: User Defined Variables are not evaluated on client (master) i… [jmeter]
Posted by "markuswege (via GitHub)" <gi...@apache.org>.
markuswege closed pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
URL: https://github.com/apache/jmeter/pull/5900
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] markuswege commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "markuswege (via GitHub)" <gi...@apache.org>.
markuswege commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1547412746
After some checking i don't think placing this in the batch tests makes sense, since we would need an assertion inside of the backend listener- The attached file was used for our manual tests to check for the correct behavior.
Also i checked the unittests. As far as i see there are currently no tests for the ClientJMeterEngine, which calls the necessary parts of the PreCompiler class change in this fix.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1547420546
Please include the tests in the PR. For now, I see only `ClientJMeterEngine` and `PreCompiler` changes.
The PRs that alter core behaviours must be accompanied by tests.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [jmeter] vlsi commented on pull request #5900: Bugfix: User Defined Variables are not evaluated on client (master) i…
Posted by "vlsi (via GitHub)" <gi...@apache.org>.
vlsi commented on PR #5900:
URL: https://github.com/apache/jmeter/pull/5900#issuecomment-1547448302
>So, why are there no tests for ClientJmeterEngine?
I have no idea.
It can be some tests are missing.
It can be the tests are executed from certain batch tests only.
It can be IntelliJ code coverage does not track tests spawned from batch tests because they execute JMeter as a custom subprocess, so they don't automatically inherit IntelliJ code coverage agent options.
---
>but i simply don't have the time to create Tests for ClientJMeterEngine and PreCompiler from scratch.
I understand what you say.
However, at the same time, I would mention that the code without tests is hard to maintain.
Imagine some time later I would start working on https://github.com/apache/jmeter/issues/5877
What would I do with the code you suggest in this PR? It would be hard to "just drop the code", however, it would be hard to understand if it still works as intended. That is one of the "maintenance overheads" we have.
The bug https://github.com/apache/jmeter/issues/5896 does not affect me directly, and I have no time for looking into it either.
So it would be great if you could reconsider and spend a little bit of time creating tests to cover your changes.
An alternative option might be waiting for somebody else to do it.
Thank you for understanding.
--
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@jmeter.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org