karate icon indicating copy to clipboard operation
karate copied to clipboard

url, path & header being passed to called feature

Open ismail-s opened this issue 1 year ago • 4 comments

Description

In Karate version 1.2.0.RC2, a bug seems to have been introduced where Karate now passes url and header (not sure if any other things as well) from a calling feature to a called feature. I've tested this with doing * def blah = call read('some.feature'), and also having an afterScenario block that does karate.call('some.feature'); both ways exhibit this bug.

The bug doesn't happen in v1.2.0.RC1 .

I can see in the Karate README that variables get passed through automatically (tbh, that feels a bit iffy to me, I thought programming languages generally completely separate called function's scope from their parent except for explicitly passed through inputs. It would be nice if we could choose to disable this feature when we didn't want it. But that's separate to this issue). But the Karate README also says https://github.com/karatelabs/karate/blob/3b4de72abf93431d8c9fc9301d806c6b931df004/README.md?plain=1#L3444

myproject.zip contains a demo of this bug (try setting the karate version to 1.2.0.RC1 & 1.2.0.RC2 to see the difference).

This bug means that e.g. if a scenario fails and an afterScenario block runs, that afterScenario block could end up making REST API calls with incorrect additional headers. Even doing * configure headers = null doesn't seem to protect against this bug, as headers set by doing * headers foo = "bar" don't seem to be able to unset in this or any other way AFAICT.

ismail-s avatar Jul 06 '22 13:07 ismail-s

@ismail-s going to take time to get to this. maybe you can contribute ;)

ptrthomas avatar Jul 07 '22 04:07 ptrthomas

We have experienced a similar issue just today where the "path" setting is accumulated/concatenated as well across feature boundaries.

I understand that path segments are expected to be concatenated; however, with 1.1.0, I was able to use different paths in a called feature, where in 1.2.0 I am not. The snippets below are not a valid "test", but show the changed (breaking) behaviour when looking at the accessed urls.

Sample: main.feature:

Feature: Main

  Background:
    * url "http://httpbin.org/get"
    * path '/main/feature'
    * call read("sub.feature")

  Scenario: Caller
    Given method get

sub.feature:

Feature: Sub

  Background: Called
    Given url "http://httpbin.org/get"
    And path '/sub/feature'

  Scenario:
    When method get

Feel free to redirect me to a different issue.

Thanks :)

bnutzer avatar Jul 07 '22 12:07 bnutzer

Yeah, that sounds the same as this. I've added it to the issue title, so others don't miss this.

ismail-s avatar Jul 07 '22 14:07 ismail-s

all: fixed. it would be great if you can validate latest develop solves this. so the only case where the http-builder can "retain state" is from the Background and for the same feature

ptrthomas avatar Aug 03 '22 09:08 ptrthomas

1.3.0 released

ptrthomas avatar Nov 02 '22 17:11 ptrthomas

I've never had an issue with the way Karate functioned in this respect, other than having to occassionally set headers to null in the few cases where I don't want them passed. I don't think we should be too rigid in the application of certain rules, if the current functionality reduces code duplication. We all use base classes right? Doesn't seem like the worst way to do something. Not saying this change should be reverted, just that there is more than one way of viewing this, as with most coding solutions ;)

adrian-85 avatar Nov 16 '22 16:11 adrian-85

@adrian-85 noted. maybe a future direction is to give users an explicit http object, which you can pass into called features. but I resist trying to make karate look like a "normal" programming language. we have gotten feedback from so many non-programmers that they love karate for keeping things simple and just reducing cognitive load. programmers tend to forget the fact that tests need to live on for years, and be maintainable in the long run, by folks new to the team, domain, etc etc.

I'm fully aware that programmers once in while will drive-by and say that things "feel a bit iffy" and whatnot, but I'll only take them seriously if they come up with constructive suggestions or best-case contribute code.

personally, I like the fact that karate encourages you to keep your "main flow" in a single feature - by making it "harder" to do "re-use" like some of the examples in this thread. I think this has worked out well to offset the urge to "re-use" and apply "design patterns" by inexperienced folks (with the best of intentions no doubt). my favorite example is this: https://stackoverflow.com/a/54126724/143475

ptrthomas avatar Nov 17 '22 03:11 ptrthomas

all I'm willing to acknowledge that url should be NOT reset ever, even when calling a feature. you can always force it by hard-coding. when I though about it, this was the original intent and it got lost in discussion and refactoring, my bad.

posted some thoughts here, do comment if you have a PoV: https://github.com/karatelabs/karate/issues/2202#issuecomment-1338690477

ptrthomas avatar Dec 06 '22 03:12 ptrthomas