Bug59945_DefaultGroovyEngineToJSR223ElementsV2

[ra0077]

Hi,

If the script language has not been chosen on the GUI, I propose to add
a WARN message into the log to inform that the script will be
interpreted as a groovy script

It's avoid message like "ERROR -
jmeter.protocol.java.sampler.JSR223Sampler: Problem in JSR223 script
Echantillon JSR223, message:javax.script.ScriptException: Cannot find
engine named: '', ensure you set language field in JSR223 Test
Element:Echantillon JSR223" and allow to execute the script

Antonio

[vlsi]

Should this be final static?

[ra0077]

Good catch

[vlsi]

Do you think a test should be added to cover the change?
An entry in changes.xml is missing as well.

[milamberspace]

The scriptEngine will be never null if groovy become the default language. This (if and exception below) must be changes.
Perhaps put here the test (if lang is undefine then log warn "interpret as groovy script")

[ra0077]

Hi Milamber,

I have keep the exception because I am not sure that getInstance().getEngineByName("groovy") will always return something.
For example if the groovy jar are missing

Antonio

[milamberspace]

The manual must be updated too for indicate this new behavior (if no language, then groovy script)

[ra0077]

Hi,

New patch with better warn message and changes.xml

I am waiting your advices for a better patch

Antonio

[milamberspace]

Seems better. Fon't forget to update the docs (manual)

[vlsi]

-1 for logging a warning on each sampler execution.
Typical user would never open log files, and get bad performance results (or even log overflow of some kind)

[ra0077]

Hi vladimir,

Can you give me more details to avoid logging a warning on each sampler execution?
I don't know how to implement

Thanks
Antonio

[vlsi]

Final static should be written in capital letters

[vlsi]

I don't know how to implement

For instance, a boolean flag that is cleared after the fist warning is printed.

[ubikloadpack]

Hello,
Thanks for PR.
My notes:

• defaultScriptLanguage should be uppercase being a constant.
• use StringUtils.isEmpty to avoid spaces breaking the control
• Maybe a solution to @vlsi note is to set the script , something like this (note that I don't think it will set the lang on the final test but only avoid repeating the log message for the same element each time it runs

`if (StringUtils.isEmpty(lang)) {

        lang = DEFAULT_SCRIPT_LANGUAGE;

        setScript(lang);

        log.warn("Script language has not been chosen on the UI: "+getName()+", the script will be interpreted as a groovy script");

    }

`

Regards
@ubikloadpack

[ra0077]

Hi ubikloadpack,

Your solution seem good

I will implement it

[ra0077]

It's don't work :-(

protected ScriptEngine getScriptEngine() throws ScriptException {
    String lang = getScriptLanguage();

    if (StringUtils.isEmpty(lang)) {
        lang = DEFAULT_SCRIPT_LANGUAGE;
        setScriptLanguage(lang);
        log.warn("Script language has not been chosen on the UI: "+getName()+", the script will be interpreted as a groovy script");
    }

    ScriptEngine scriptEngine = getInstance().getEngineByName(lang);
    if (scriptEngine == null) {
        throw new ScriptException("Cannot find engine named: '"+lang+"', ensure you set language field in JSR223 Test Element: "+getName());
    }

    return scriptEngine;
}

[pmouawad]

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 don't want to PR again.

Thanks

[ra0077]

Hi all,

I have check in JMeter source code if this "problem" (take a default solution if nothing is setted in GUI) and I have find "CSS/JQuery Extractor" element

If you check the documentation (https://jmeter.apache.org/usermanual/component_reference.html#CSS/JQuery_Extractor), we can read "If selector is set to empty, default implementation(JSoup) will be used"

I have check the source code and make a test and when selector is set to empty, there is no log written.

I propose to make the same thing to this PR (remove log to avoid performance problem + have a clear documentation)

Are you ok?

Antonio

[vlsi]

remove log to avoid performance problem

+1

Technically speaking, I would prefer to set the language to "groovy" if it was not specified.

That would make the UI clear (user will see what the language is), and that would setup proper syntax highlight mode (much easier to program).
If the conversion is done at load time, then additional log warning is just fine as well.

[milamberspace]

OK for me too.

[ra0077]

PR commited