Bug 61640 - JSR223 Test Elements : Enable by default caching
Summary: JSR223 Test Elements : Enable by default caching
Status: RESOLVED FIXED
Alias: None
Product: JMeter
Classification: Unclassified
Component: Main (show other bugs)
Version: 3.3
Hardware: All All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: JMeter issues mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-10-22 17:02 UTC by Philippe Mouawad
Modified: 2017-11-17 21:42 UTC (History)
2 users (show)



Attachments
Patch with change default selected and add variable warning (6.63 KB, patch)
2017-11-09 14:08 UTC, orimarko
Details | Diff
Patch for JSR cache checkbox on by default (6.91 KB, patch)
2017-11-12 08:22 UTC, orimarko
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Philippe Mouawad 2017-10-22 17:02:34 UTC
As defaults should be the best options and knowing that:
- not checking this option when using a Compilable language makes performances bad
- using ${var} syntax in dynamic language makes also performances bad

This option should be checked by default and an advice on using vars.get("varName") instead of ${varName} should be displayed
Comment 1 orimarko 2017-11-09 14:08:15 UTC
Created attachment 35511 [details]
Patch with change default selected and add variable warning

Patch with change default selected and add variable warning
Comment 2 Philippe Mouawad 2017-11-09 21:00:52 UTC
T(In reply to orimarko from comment #1)
> Created attachment 35511 [details]
> Patch with change default selected and add variable warning
> 
> Patch with change default selected and add variable warning

Thanks for patch, but it is not working for me.
When I add a JSR223PreProcessor, field is not checked.

Regards
Comment 3 orimarko 2017-11-12 08:22:40 UTC
Created attachment 35521 [details]
Patch for JSR cache checkbox on by default

Patch for JSR  cache checkbox on by default
Comment 4 Philippe Mouawad 2017-11-12 10:14:38 UTC
Thanks for updated patch.
But unfortunately, although there is some improvement, it is still not working for me:
- Add a JSR223 Pre Processor 1, it is checked => OK
- Add a JSR223 Pre Processor 2, uncheck=> OK
- Go to JSR223 Pre Processor 1, it is checked => OK
- Go to JSR223 Pre Processor 2, it is checked => KO


Do you confirm issue on your side ?
Thanks
Comment 5 orimarko 2017-11-12 11:02:24 UTC
Yes, There's an issue with my patch with more than 1 element
Comment 6 orimarko 2017-11-12 12:24:17 UTC
I'm found a RESOLVED FIXED issue #56554 which maybe make the checkbox irrelevant because it generate cache key automatically depending only on Compilable without connection to cache key check box

So maybe we need to remove checkbox?
Comment 7 orimarko 2017-11-12 12:54:04 UTC
In code if there's a script file it ignores cacheKey from UI, only if it's inline script it just check if not empty cacheKey and ignore the value from UI
Comment 8 Philippe Mouawad 2017-11-12 14:09:33 UTC
(In reply to orimarko from comment #7)
> In code if there's a script file it ignores cacheKey from UI, only if it's
> inline script it just check if not empty cacheKey and ignore the value from
> UI

The checkbox is still relevant.
See in JSR223TestElement:
if (supportsCompilable && !StringUtils.isEmpty(cacheKey)) {


The checkbox is here to allow people who want to use Variables in their script to do it, mainly for backward compatibility.
Comment 9 Philippe Mouawad 2017-11-17 21:42:37 UTC
Author: pmouawad
Date: Fri Nov 17 21:42:18 2017
New Revision: 1815632

URL: http://svn.apache.org/viewvc?rev=1815632&view=rev
Log:
Bug 61640 - JSR223 Test Elements : Enable by default caching
Bugzilla Id: 61640

Modified:
    jmeter/trunk/src/core/org/apache/jmeter/util/JSR223TestElement.java
    jmeter/trunk/src/core/org/apache/jmeter/util/ScriptingBeanInfoSupport.java
    jmeter/trunk/xdocs/changes.xml