Request to execute a scenario executes entire feature file

[rwong-gw]

Karate 1.3 introduced a bug
where it would sometimes execute an entire feature file when it was only supposed to execute a single scenario within
the feature file. We are working around this bug by always using a Karate PerfHook
to execute scenarios.

Attached is a zip file with a test (KarateScenarioTest.java) that reproduces the issue.

karate-scenario-execution-bug.zip

[ptrthomas]

@rwong-gw that was to fix #2034 - so I won't say it is a bug.

the best option is for you or someone else on your team to submit a PR. since no one else has reported this, I'll tag this issue as help wanted

[rwong-gw]

Well, when a bug is introduced by fixing another bug, I would still say it's a bug.

Since I don't understand the use case that was fixed, and that code has a predicate that IMO is trying to do too much in one line (if (callTag != null && (!fr.caller.isNone() || fr.perfHook != null)) ...), and we already have a workaround (always use a non-null PerfHook), I doubt my team will be submitting a fix for this.

[ptrthomas]

@rwong-gw we regularly release RC versions to get early feedback for exactly the kind of "bug" you are reporting, but I understand - not all teams have the bandwidth to do that. as an open source project, we don't have that much bandwidth either

I'll leave this open for a while and hope someone from the community contributes

[AKushWarrior]

@ptrthomas I did some digging; it looks like that particular guard is unnecessary, as the intervening 2 years have seen some regression blockers put in; moving (!fr.caller.isNone() || fr.perfHook != null)) to the top-level conditional (or deleting that boolean condition entirely) doesn't regress to the problem described in #2034. In fact, with both of those two tweaks, the unit test for call-self.feature still passes.

However, reverting to the old (pre-Karate 1.3) behavior doesn't actually solve the issue that rwong-gw reported; the problem is that since there is no caller and no hook, the library (as constructed both pre and post-1.3) doesn't believe in allowing call-by-tag of a single scenario. The only way to solve the issue, based on my fairly extensive testing, is to simply delete the (!fr.caller.isNone() || fr.perfHook != null)) entirely.

In that fairly extensive testing (including translating rwong-gw's original example into a unit test), I found that deleting the guard entirely doesn't seem to introduce any regressions. Based on that, I opened a PR (#2505) with that block deleted. If you dislike that choice as a design decision, feel free to reject it – I just don't think there's any other way to solve this particular issue.

[ptrthomas]

@AKushWarrior thanks, really appreciate the research. accepted the PR and your recommendation. we'll see how it goes when we release

[rwong-gw]

@AKushWarrior thank you for the PR. May I suggest an improvement to FeatureRuntimeTest#testSingleScenario?

Could you add the assertions:

        matchContains(result.getVariables(), "{ result1: '#notpresent' }");
        matchContains(result.getVariables(), "{ result3: '#notpresent' }");

Since the reason for the bug was that Karate was executing other scenarios besides the one we requested, adding these assertions will ensure that this test only executed @Scenario2.

[AKushWarrior]

@rwong-gw @ptrthomas Whoops, good catch. The test still passes (as expected) after this change; since the first PR was already merged, I just submitted a second one (#2506) to fix the test. Sorry about the oversight.