Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues calling .feature file and java code #544

Closed
connorsadlervelo opened this issue Oct 2, 2018 · 20 comments
Closed

Issues calling .feature file and java code #544

connorsadlervelo opened this issue Oct 2, 2018 · 20 comments
Assignees

Comments

@connorsadlervelo
Copy link

connorsadlervelo commented Oct 2, 2018

Hi,
Couple of issues found while calling out to .feature files and also to Java code
Please see attached zip file for reproducible
3 scenarios are in cats-java.feature
You can run them using: CatsJavaRunner
(these files were cribbed from examples as you'll probably recognise)

Will paste in the cats-java.feature below to show the 3 scenarios in the ticket - please see zip for full code and dependencies.

Thanks,

Connor


Feature: cats end-point

Background:
* def JavaDemo = Java.type('com.intuit.karate.demo.util.JavaDemo')

#
# Expected: We should be available to access "karate.prevRequest.headers" in the called "showPrevRequestHeaders.feature" file
# Actual:   We cannot access these headers
#
Scenario: Issue 1 - Make a request and then use karate prevRequest headers - works only in this file, not in a called feature file
    * url 'https://jsonplaceholder.typicode.com'
    Given path 'users'
    When method get
    Then status 200
    * def first = response[0]
    Given path 'users', first.id
    When method get
    Then status 200
    * def prevRequest_headers = karate.prevRequest.headers
    * print "prevRequest_headers: " + prevRequest_headers + "\n"
    * def prevRequest_item = karate.prevRequest.headers['Accept-Encoding'][0]
    * print "prevRequest_item: " + prevRequest_item + "\n"
    * call read("showPrevRequestHeaders.feature")

#
# Expected: The Cucumber report should work OK
# Actual: Please run this using CatsJavaRunner.java
#         I see an exception: java.lang.RuntimeException: no Getter for field logAppender in class com.intuit.karate.Logger
#         This appears to be the report attempting to serialize the "karate" value as JSON, which I don't need it to do
#
Scenario: Issue 2 - pass object to feature file - causes error at Report time when deriving JSON representation of values
  * def myClassInstance = JavaDemo.createInstanceOfMyClass()
  * call read("mylogic.feature") { outerKarate: #(karate), myClassParam: #(myClassInstance) }

#
# Notes: Maybe I am just not understanding the syntax here - can I use 'call' to call a Java method?
# Expected: The call to myClassInstance.myVoidMethod "FAIL" should call the java code and throw an exception, and the test should fail
# Actual: I see WARN messages in the log
#         e.g.  WARN  com.intuit.karate - not a js function or feature file: myClassInstance.myVoidMethod - [type: UNKNOWN, value: [jdk.internal.dynalink.beans.SimpleDynamicMethod void com.intuit.karate.demo.util.MyClass.myVoidMethod(String)]]
#               WARN  com.intuit.karate - not a js function or feature file: myClassInstance.myVoidMethodxxx - [type: NULL, value: null]
#
Scenario: Issue 3 - How to call a Java method with a void return value using call without def
    * def myClassInstance = JavaDemo.createInstanceOfMyClass()

    # try calling some 'String' methods, this works OK
    * def resultOfMyStringMethod = myClassInstance.myStringMethod("hello")
    * print "resultOfMyStringMethod: " + resultOfMyStringMethod
    * def resultOfMyVoidMethod = myClassInstance.myVoidMethod("hello")
    * print "resultOfMyVoidMethod: " + resultOfMyVoidMethod

    # try calling some 'void' methods - not sure of syntax here
    # I was expecting a failure at this point in the test rather than WARN messages
    # doesnt seem to call this method -> WARN in output
    * call myClassInstance.myVoidMethod "FAIL"
    # doesnt seem to call this method either - method doesnt exist -> WARN in output
    * call myClassInstance.myVoidMethodxxx "hello world"

@ptrthomas ptrthomas self-assigned this Oct 2, 2018
@ptrthomas ptrthomas added this to the 0.9.0 milestone Oct 2, 2018
ptrthomas added a commit that referenced this issue Oct 2, 2018
@ptrthomas
Copy link
Member

thanks for the detailed example:

  1. you can pass the prevRequest object to the called file after the fix in 0.9.0. for now as a workaround, you can pass the headers and other details separately one at a time. because there is a JSON serialization bug
    * def prevRequest = karate.prevRequest
    * call read("showPrevRequestHeaders.feature") { prevRequest: '#(prevRequest)' }
  1. sorry we'll never allow karate to be passed around to features. you could do some interesting things in java if you what you are doing, here is an example (0.9.0): https://github.com/intuit/karate/tree/cukexit/karate-gatling#custom

  2. call is just syntax convenience designed for single arg situations for JS and features only. the below work, so is fine IMO.

    * def resultOfMyVoidMethod = myClassInstance.myVoidMethod("hello")
    * eval myClassInstance.myVoidMethod("foo")

@ptrthomas ptrthomas added the fixed label Oct 2, 2018
@connorsadlervelo
Copy link
Author

connorsadlervelo commented Oct 2, 2018

Thanks for the reply.

Really 1+2 are related.
Our called logic needs access to the karate.prevRequest headers.
I can pass "karate" by using the workaround code below to avoid the JSON serialization issue.
I also need this workaround for another Java variable which we need to pass in, which gives the same serialization problem. I don't really need or want the value turned into JSON at all, but the report seems to do that in order to show the value in the output. The workaround code just puts dummy values in the output.
Is this workaround code OK to use, or should I look to restructure it somehow?
I don't really want to recode in Java as then we lose the syntax of .feature files.

// Work around problems in report when java/javascript values have been passed into .feature files e.g. karate variable, a QueueConsumer variable
// The Karate report code tries to show a JSON representation of every parameter value passed into a called .feature.
// This JSON rep derivation will fail on certain types, including the karate variable and an instance of our QueueConsumer
// (the failure is because of missing getter methods for certain fields)
// We are completely uninterested in this json representation in the report, so we want to just override it with an arbitrary string
// Thankfully, after checking the source code, we can see we can install an override "JSON representation deriver" for the appropriate classes
JSONValue.defaultWriter.registerWriter(new MyKarateWriter("ScriptBridge aka karate variable"), ScriptBridge.class);
JSONValue.defaultWriter.registerWriter(new MyKarateWriter("QueueConsumer aka a messageListener variable"), QueueConsumer.class);

/**
 * Writes a dummy json representation, including the specified description
 */
private static class MyKarateWriter implements JsonWriterI {
    private String description;

    public MyKarateWriter(String description) {
        this.description = description;
    }

    @Override
    public void writeJSONString(Object value, Appendable out, JSONStyle compression) throws IOException {
        out.append("[[MyKarateWriter writing json representation of " + description + "]]");
    }
}

(3) the syntax confused me, IMHO it could be worth changing the WARN to an error of some sort.
The WARN messages were getting lost in the logging for me.

@ptrthomas
Copy link
Member

I don't really want to recode in Java as then we lose the syntax of .feature files.

I'm going to disagree here based on experience. if you are doing something so complex, move it into Java or at least JS. I'm now dealing with a team that has so much in JS and "called" features that they are not able to debug and troubleshoot problems.

so I'm inclined not to support this unless we get a PR or something.

actually if you do a * def foo = bar - foo will be seen by called features without you needing to do anything. see if that gets you somewhere without needing to serialize JSON.

@ptrthomas
Copy link
Member

one more thing I really discourage calling features except in cases where you do a common "setup".

@connorsadlervelo
Copy link
Author

I thought calling .feature files was the way to go
I can indeed pass arguments by listing them all on separate lines in def statements, but the script becomes long - if we have 4 arguments it's 5 lines for each call instead of one :(.
If I have to make the call 3 times then that's 15 lines instead of 3

What sort of PR would you be interested in?

@ptrthomas
Copy link
Member

@connorsadlervelo you can pass a JSON in one line so your args are name-spaced.

* def foo = { bar: 1, baz: 2, ban: 3 }

any other questions ?

@connorsadlervelo
Copy link
Author

I am not sure of the overall recommendation here
Is your recommendation to call .feature files, and instead of passing in args we should use a known json variable such as "* def foo = { ... args ... }" ?
I take it this means Karate will not attempt to serialize the args as JSON for the report.
This seems like a workaround to me - the docs say you're not limited in any way!

Is the only other option to avoid this completely and call Java or Javascript?
If that's the way forward, would it be possible to add to the karate object the ability to mimic a "* json ..." command? Or is that there already and I've missed it?
That's the part where I wanted to use a .feature file for over Javascript.

Thanks for the help so far!

@ptrthomas
Copy link
Member

workaround for YOUR specific edge-case which is not recommended. IMO a very reasonable workaround - but you can choose to disagree.

you can use a Java utility to do this "mimic json" you speak of if it is not already covered here: https://github.com/intuit/karate#the-karate-object

@connorsadlervelo
Copy link
Author

BTW this is where I got the original recommendation from - see your answer on stack overflow.
https://stackoverflow.com/questions/52541972/calling-a-feature-and-passing-in-javascript-variables/52542039#52542039

I know I could use another library to do the json logic, I'd rather use karate functions if possible.

@ptrthomas
Copy link
Member

we are going in circles here. the problem is you want karate to be passed around which I don't recommend.

@connorsadlervelo
Copy link
Author

Not at all, I'd rather not pass it anywhere, that was itself a workaround.
I need our called feature to be able to access the karate.prevRequest from the calling .feature file
Would that be feasible / possible?

BTW the report "error on JSON serialization of args" issue - that problem occurred with another of our Java objects, not just the karate one.

@ptrthomas
Copy link
Member

Would that be feasible / possible?

Already answered in my first comment in this thread. I have nothing more to add.

another of our Java objects, not just the karate one.

As per design. You already know normal JSON is fine. BTW Java Map-s and even valid JavaBeans (with proper getters / setters) will work, but your mileage may vary.

@connorsadlervelo
Copy link
Author

connorsadlervelo commented Oct 3, 2018

Sigh, getting nowhere here. I give up.

For people finding this thread in future
My conclusions are:

EDIT by [peter] --> @ptrthomas | responses inline.

  1. The karate.prevRequest does not work correctly in a called .feature file i.e. it does not see the previous request made

[peter] exactly as per design. each feature opens a new scope without which karate would break in all kinds of ways. in just one extra line you can save the value of karate.prevRequest and pass it around.

  1. Contrary to the docs, Karate does limit us regarding values we pass between feature files. Workarounds rather than fixes have been proposed in here.

[peter] wow, that's such a cheap shot considering the amount of effort that has gone into the documentation. I'll leave it to readers to decide the merit of your accusation. and of course this is in a situation which we do not support (which is passing the karate reference to called feature files)

  1. I'm still not sure whether the Karate dev's recommendation is to write reusable logic in .feature files, or in Java or Javascript code

[peter] simple. for complex stuff which you seem to want to do (framework-y stuff like storing the previous values of request headers) please use Java for maintenance and debug-ability reasons. we recommend that re-usable features be used sparingly and only for "set-up" routines, but many teams break this rule without any problems.

  1. I do not know if I can replicate a "* json" command easily in Java or Javascript, making it difficult to convert a .feature file to a Java or Javascript block

[peter] I have NO idea what you are talking about. wild guess: when you are going to java, you shouldn't worry about JSON anymore because you will get the object as-is

@ptrthomas
Copy link
Member

I'm keeping this open because there is a fix for passing karate.prevRequest to called files that will be part of 0.9.0 final.

@ptrthomas ptrthomas reopened this Oct 3, 2018
@connorsadlervelo
Copy link
Author

Fair enough.

@ptrthomas
Copy link
Member

@connorsadlervelo lol, didn't notice your sneaky ninja edit :) I've edited your comment with my responses to your conclusions.

@connorsadlervelo
Copy link
Author

connorsadlervelo commented Oct 4, 2018

Peter, IMHO you are taking this far too personally.
Karate as a whole I have found reasonably good, and the docs are very good, but this code reusability part could be improved - again IMHO.
You should want people to use your framework and be prepared to address such issues. Frankly you do not seem open to that at all.

Editing my response has cluttered the conversation and made it difficult for me to reply.

There were no "cheap shots" at all there. I was stating factually my observations and conclusions. Your documentation does state that we are not limited, but it appears there are limits there.
Again, I do not wish to pass "karate" around at all.

Your own response on Stack Overflow originally led me down the .feature path. Do you think you should go back and edit that, if your advice has changed?

@ptrthomas
Copy link
Member

ptrthomas commented Oct 4, 2018

@connorsadlervelo

Peter, IMHO you are taking this far too personally.

Nope, if you make sweeping accusations it is my responsibility to provide my PoV. And I'm going to ignore the accusations you continue to make above for the sake of ending this discussion sooner. Again - let the readers decide, based on what they have seen here in the other issues and on Stack Overflow.

Editing my response has cluttered the conversation and made it difficult for me to reply.

I do have to remind you here that it was you who made an edit to an old comment - so I never realized that you had injected a whole bunch of accusations for the world to read.

Do you think you should go back and edit that, if your advice has changed?

IMO that is a general question where feature re-use made sense, I could be wrong though :)

@connorsadlervelo
Copy link
Author

"Accusations"?
I spent my time giving you a concrete simplified reproducible of all the issues I've seen.
I've never made any "accusations".
As I say, you're taking this very personally.
Now giving up again!

@ptrthomas
Copy link
Member

I'll make one last point as I lock this thread as an administrator. What really ticked me off here is the continued reference to the docs say you're not limited in any way and using that as FUD.

The docs say no such thing.

There is this statement though completely in the context of re-usable Java code:

re-use custom Java code if needed, for ultimate extensibility

Peace.

@karatelabs karatelabs locked as too heated and limited conversation to collaborators Oct 4, 2018
@ptrthomas ptrthomas removed this from the 0.9.0 milestone Nov 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants