Prevent loop when callonce is calling itself

[tom08zehn]

I'm working with karate-1.0.1.jar and calling it as follows:

java -jar karate-1.0.1.jar -C -T 1 my.feature

Not sure if I found a bug or if this is a feature request.

The goal is to define re-usable variables (shared across scenarios within the same feature) in the same file and not to outsource them to another file for improved readability and better user experience. This is especially useful if you just need one (or a few) shared variables.

The idea is to tag a scenario as @ignore and at the same time @bar. In the feature's Background you use callonce to run just the feature tagged with @bar:

Feature: My

Background:
* def foo = callonce read('my.feature@bar')

@ignore @bar
Scenario: Return shared variables for all scenarios
    * def id = String(java.util.UUID.randomUUID())

@one
Scenario: One
    * print foo

@two
Scenario: Two
    * print foo

Currently this results in an infinite loop and finally an error:

[main]  INFO  c.intuit.karate.core.FeatureRuntime - scenario called at line: 7 by tag: @bar
[main]  INFO  com.intuit.karate - >> lock acquired, begin callonce: read('my.feature@bar')
...
[main]  ERROR com.intuit.karate - my.feature:4
* def foo = callonce read('my.feature@bar')
java.lang.StackOverflowError
...
[main]  WARN  com.intuit.karate.JsonUtils - object to json serialization failure, trying alternate approach: null
[main]  ERROR com.intuit.karate.Suite - <<error>> unable to write report file(s): my.feature - java.lang.StackOverflowError

--

I think I found a way to make this work but unfortunately I'm not used to do Java programming. Please take this as a proposal: I added 2 new code snippets:

Original here (master): https://github.com/intuit/karate/blob/master/karate-core/src/main/java/com/intuit/karate/core/ScenarioEngine.java#L1971

    private Variable callOnce(String cacheKey, Variable called, Variable arg, boolean sharedScope) {
        final Map<String, ScenarioCall.Result> CACHE = runtime.featureRuntime.FEATURE_CACHE;
        ScenarioCall.Result result = CACHE.get(cacheKey);
        if (result != null) {
            logger.trace("callonce cache hit for: {}", cacheKey);
            return callOnceResult(result, sharedScope);
        }
        
        
        // *** new 1: begin
        // return if cache key exists (don't call again)
        if (CACHE.containsKey(cacheKey)) {
            logger.trace("callonce empty cache for: {} (don't call again)", cacheKey);
            return result;
        }
        // *** I'm not sure what in detail happens when returning null here
        // *** new 1: end
        
        
        long startTime = System.currentTimeMillis();
        logger.trace("callonce waiting for lock: {}", cacheKey);
        synchronized (CACHE) {
            result = CACHE.get(cacheKey); // retry
            if (result != null) {
                long endTime = System.currentTimeMillis() - startTime;
                logger.warn("this thread waited {} milliseconds for callonce lock: {}", endTime, cacheKey);
                return callOnceResult(result, sharedScope);
            }
            // this thread is the 'winner'
            logger.info(">> lock acquired, begin callonce: {}", cacheKey);
            
            
            // *** new 2: begin
            // before doing something, create key in cache so that `CACHE.containsKey(cacheKey)` returns true 
            CACHE.put(cacheKey, null);
            // *** new 2: end
            
            
            Variable resultValue = call(called, arg, sharedScope);
            // we clone result (and config) here, to snapshot state at the point the callonce was invoked
            // detaching is important (see JsFunction) so that we can keep the source-code aside
            // and use it to re-create functions in a new JS context - and work around graal-js limitations
            Map<String, Variable> clonedVars = called.isFeature() && sharedScope ? detachVariables(true) : null;
            Config clonedConfig = new Config(config);
            clonedConfig.detach();
            Object resultObject = recurseAndDetachAndDeepClone(resultValue.getValue());
            result = new ScenarioCall.Result(new Variable(resultObject), clonedConfig, clonedVars);
            CACHE.put(cacheKey, result);
            logger.info("<< lock released, cached callonce: {}", cacheKey);
            return resultValue; // another routine will apply globally if needed
        }
    }

--

For even more readability it would be great if callonce read() would not only support a feature or JavaScript file but also just simply a tag. Then you could simply do:

Background:
* def foo = callonce read('@bar')

... which means that the scenario @bar of the same file is called. This would be a fancy shortcut to:

Background:
* def me = String(context.getFeatureContext().feature.getPath().toFile().getName().toString())
* def foo = callonce read(me + '@bar')

[ptrthomas]

@tom08zehn thanks, but this is absolutely not a priority for this project. implementing call and getting it to "play nice" with callonce has been hard.

so this is being closed as a wontfix - that said, you or anyone reading this is most welcome to do the research and contribute code

[tom08zehn]

@ptrthomas Thanks anyway.

That's exactly what I tried: contributing the code.
I'm just not expert enough to review if this would break something else.

I would really like to try it on my own but am not used at all to work with Java source code... maybe somebody else finds this useful and can do a code review - or I can propose a merge request?

If not, then I'm sorry to have bothered you with a "nice to have" feature.

[ptrthomas]

That's exactly what I tried: contributing the code.

@tom08zehn and I appreciate that. the ideal code contribution for us (and perhaps most open-source projects) is that you at the very least run all the tests. it is indeed unfortunate that you don't have experience with java code.

there are others who may be watching the project discussions, so let's hope someone steps in to help.

a bit of context - we have been fighting a very hard problem related to callonce for months now, refer #1558 - and that is one of the reasons this falls very low on our priority list