karatelabs / karate

Test Automation Made Simple
https://karatelabs.github.io/karate
MIT License
8.08k stars 1.94k forks source link

replace graal with karate-js engine #2546

Open ptrthomas opened 5 months ago

ptrthomas commented 5 months ago

refer: https://github.com/karatelabs/karate-js for full background

this will give us the following advantages:

ptrthomas commented 5 months ago

the build passes on github-actions (that too on Java 23 EA) 🎉

this is a very big deal ! tagging a few power users, please do try this on your local test suites when you get a chance, the thing to look out for is if you have used any JS apis or tricks that we don't yet support @edwardsph @ericdriggs @joelpramos @brown-kaew @fabio-andre-rodrigues @rwong-gw @cl-weclapp @AKushWarrior @bouncysteve @bondar-artem @f-delahaye @dvargas46 @maxandersen @arnault01 @jkeys089 @bischoffdev @staffier

image

this is the issue that tipped me over the edge: #2536

full changeset: https://github.com/karatelabs/karate/commit/ba931c8a5b9c1690f923638ff34fe27167f68eee

maxandersen commented 5 months ago

very interesting - will try it out.

I have to ask - why not just use and possibly fork nashhorn standalone? https://github.com/openjdk/nashorn ..maybe as an option?

anyhow - just curious. will see if i can make karata with this PR run jbang testsuite in near future.

ptrthomas commented 5 months ago

@maxandersen pretty sure Nashorn does not support some ES6 features - the big one being arrow-functions. I could be wrong on specifics, but it certainly fails the small set of ECMA conformance tests that we tried. Interestingly there are some folks who are keeping Rhino alive - for e.g. the HTMLUnit project who are now looking for new engine.

ericdriggs commented 5 months ago

Thank you for directly addressing this risk. I look forward to the potential improvements in speed, performance, concurrency, flakiness, and reduced maintenance and reduced frequency of breaking js engine changes.

Excellent start.

woess commented 4 months ago

pretty sure Nashorn does not support some ES6 features - the big one being arrow-functions

Basic arrow functions are supported (in --language=es6 mode).

jbalme commented 3 months ago

Wait, what?

This is going to break things horribly for anyone using anything from modern JavaScript on Karate.

Modern JS semantics were a huge selling point for the framework for me.

Are you going to work with babel etc... so people can at least compile modern JS down to something the new engine can run?

This has me leaning in the direction of just reimplementing the karate keywords in cucumber-jvm and graaljs.

ptrthomas commented 3 months ago

@jbalme your concerns are noted ;) I think the whole history of WHY this change was needed in the first place makes the need very clear: https://github.com/karatelabs/karate-js

in my opinion - based on 7 years of looking at tests created by users - karate-js supports 100% of what teams need. proof is this set of tests - all of which run successfully on the new engine: js-arrays.feature

anyone is welcome to contribute any missing syntax, if you are wondering how hard that may be - take a look at this commit: added do-while syntax

but sure, I understand if you want to go some other route. all the best !

jbalme commented 3 months ago

With all due respect, having something like do-while which is a JS feature that has been there since before JS even became a standard only implemented last week isn't something that inspires confidence.

Is there a reason you decided to implement a custom dialect of JS instead of eg forking Nashorn or seeing if migrating to another language with performance characteristics that might better suit what you need? Creating a new implementation of a language with as much complexity as JS is not something to be taken lightly.

ptrthomas commented 3 months ago

@jbalme IMHO all these questions have been addressed in the links above. I have nothing more to add :)

ivangsa commented 3 months ago

With all due respect, having something like do-while which is a JS feature that has been there since before JS even became a standard only implemented last week isn't something that inspires confidence.

👍

ptrthomas commented 3 months ago

With all due respect, having something like do-while which is a JS feature that has been there since before JS even became a standard only implemented last week isn't something that inspires confidence.

👍

😆

ericdriggs commented 3 months ago

@jbalme Graal does not support concurrent threads, which is a pretty big deal in the JVM. Graal design results in

The right body to address this criticism to is the ECMAScript TC39, in charge of the ECMAScript/JavaScript specification. JavaScript, by design, is specified with single-threaded evaluation semantics. Unlike other languages, it is NOT designed to allow full multi-threading.

https://github.com/karatelabs/karate-js?tab=readme-ov-file#the-problem

jbalme commented 3 months ago

Graal isn't perfect, but how is creating a brand new intentionally incomplete JS engine a better solution than picking one of the other mature JS engines on JVM and maintaining it? Nashorn doesn't have the issues with concurrency you describe, is mature and fully ES5 compliant with most of ES6 already done, and widely used in the Java ecosystem even though it is now no longer officially supported by Oracle. Is it licensing concerns with GPL+classpath of Nashorn?

I know you highlighted CVEs as a concern with Graal, I'm sure you're aware that just because you have less CVEs doesn't mean you have a more secure system - it could just be less people doing security audits on your code so issues aren't found. That will almost certainly be the case here.

jbalme commented 3 months ago

OT:

Personally, my stake in this is that I have a large amount of utility functions dependent on modern JS features (some including pulling in third-party JS libraries like fakerjs and chai) that you have highlighted as explicitly not planned, so I'm basically forced to either stay on 1.4.1 forever (not really tenable), rewrite everything in Java, or rewrite everything for another framework entirely.

My org has been skeptical of Karate which makes doing the latter and submitting to the internal pressure to do everything in Java + RestAssured the easiest choice sadly.

ptrthomas commented 3 months ago

@jbalme it would be more constructive if you pointed out what you use currently which is not supported. if there is internal pressure to use rest-assured, from experience I think you should just do that. this is because such teams use karate "in anger" and will blame any little thing on karate. I sincerely wish you find the solution that works for you and your team. all the best

jbalme commented 3 months ago

karate-js doesn't seem to have any of the JS built-in prototype methods, even for eg. Array or Object

eg. for all of the below:

[1,2,3].forEach(i => {})
Object.fromEntries(Object.entries({k: 'v'}).map(([k,v]) => [v,k]))

I get:

Exception in thread "main" io.karatelabs.js.EvalError: java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "object" is null
(inline)
    at io.karatelabs.js.Engine.evalInternal(Engine.java:70)
    at io.karatelabs.js.Engine.eval(Engine.java:51)
    at org.example.Main.main(Main.java:19)

Something I use a lot of in tests is Array.prototype.some and Array.prototype.every:

* assert ['somearray'].every(some_method())
* assert ['somearray'].some(other_method())

I'm assuming prototypes for builtins don't exist at all at the moment because it's trying to look it up and failing?

I used modules, but at least if you had most of ES5's prototypes for the basic types, I could probably deal with removing those via Babel, but as-is, karate-js doesn't really seem to support anything but basic syntax and Java object creation.

jbalme commented 3 months ago

Should I file issues on karate-js regarding these? Or would they be out of scope?

I will see if I can be allowed to implement most of what I need myself on my own time, but I'm not 100% sure what my employer's policy on contributions to things adjacent to work are so I need to get approval first.

ptrthomas commented 3 months ago

karate-js doesn't seem to have any of the JS built-in prototype methods, even for eg. Array or Object

no, it has the most common ones. the plan is indeed to add all of them, because this is important for JSON manipulation.

EDIT - global Object and Array stuff is here: https://github.com/karatelabs/karate-js/blob/ba94812596fcfd81e9eedf28086c818a8df49727/karate-js/src/main/java/io/karatelabs/js/JsCommon.java#L103

yes, we will never do modules or require. By the way we never encouraged this kind of JS, you will not find a single official example or in the docs - and we have been very consistent about this: https://stackoverflow.com/a/52100077/143475

yes - please raise issues on the karate-js issues tracker for Arrays / Object

ericdriggs commented 3 months ago

@jbalme

Is there a reason you decided to implement a custom dialect of JS instead of eg forking Nashorn

Existing libraries include combinations of

  1. deprecated/unmaintained
  2. they don't support multithreading

Difficulty in maintenance is a direct result of large language scope.

"With the rapid pace at which ECMAScript language constructs, along with APIs, are adapted and modified, we have found Nashorn challenging to maintain." https://openjdk.org/jeps/335

Nashorn hass been in maintenance mode since 2021 https://github.com/openjdk/nashorn/commits/main/

ptrthomas commented 3 months ago

actually since @jbalme gave me so much grief for that "do while" commit, I have a challenge for him. please count how many lines of code in Nashorn or Rhino or Graal is the equivalent of the "do while" in karate-js and report your findings back :)

joelpramos commented 3 months ago

@jbalme it would be more constructive if you pointed out what you use currently which is not supported. if there is internal pressure to use rest-assured, from experience I think you should just do that. this is because such teams use karate "in anger" and will blame any little thing on karate. I sincerely wish you find the solution that works for you and your team. all the best

logic isn't always the primary reason beyond decisions in an enterprise

Wondering whether a path forward is to have an abstraction that gives users the ability to import a graal or karate-js dependency (later being the default) and you explicitly deprecate / LTS the usage of Graal with Karate but hold to its compatibility of features as of today while not supporting new "issues" derived from multi-threading etc. (basically just bump up versions for CVEs). Just keep a separate build step in the pipeline to ensure tests don't break / ignore for new features (and explicitly mark as non-supported).

This gives users of karate the power of choice and avoids a lock in of a decision into karate-js while minimizing (to the degree of possible) the additional maintenance effort you currently have to work around the limitations that Graal brings to the karate ecosystem and potentially avoiding problems such as @jbalme is mentioning and allowing time for a gradual move away from Graal by users.

Obviously, an abstraction is easier said than done but I think the "old way" you had with the JsEngine, JsValue etc. was a good blueprint to get that going.

ptrthomas commented 3 months ago

@joelpramos yes, a pluggable JS engine is ideal, just that we don't have the bandwidth for it unless the community contributes. I think you and @ericdriggs know the amount of pain it was to get graal to behave.

I strongly feel that karate-js being a subset of "real JS" is a very VERY good thing. for example it would prevent teams from making silly mistakes like using chai.js when karate's JSON assertions are so powerful. so many teams get into trouble because they over-used JS, and over-complicated their tests making karate tests un-readable. and then karate gets blamed. keeping karate simple and reducing the number of ways users can shoot themselves in the foot is in my opinion a HUGE win, even if it means some users get upset - hey you can't please everyone. I have been observing karate usage in the wild for years, and have a lot more to say on this - but I'll stop here

ericdriggs commented 3 months ago

People think focus means saying yes to the thing you've got to focus on. But that's not what it means at all. It means saying no to the hundred other good ideas that there are. You have to pick carefully. I'm actually as proud of the things we haven't done as the things I have done. Innovation is saying no to 1,000 things. (Steve Jobs)

I can blame graal directly for about half the issues I've raised with karate. A high percentage of those graal issues were non-deterministic concurrency issues, which required a fair bit of effort both to replicate reliably and to fix.

jbalme commented 3 months ago

actually since @jbalme gave me so much grief for that "do while" commit, I have a challenge for him. please count how many lines of code in Nashorn or Rhino or Graal is the equivalent of the "do while" in karate-js and report your findings back :)

LoC is an absolutely terrible benchmark 99% of the time. If you really want to minimize LoC though, design the engine such that many things can be implemented in "JS" itself, JS is a lot more compact than Java. I assume you have your reasons for not doing that.

@joelpramos

Hit the nail on the head. I know I'm owed nothing, but migrating away from "real JS" to a DSL is the kind of thing that needs some kind of communication outside of issue trackers and Twitter so people at least can try and plan for the direction the framework is heading.

I strongly feel that karate-js being a subset of "real JS" is a very VERY good thing.

Then I would recommend rebranding it away from the name JS so people don't get the wrong idea, and explicitly documenting the new DSL giving a list of what is and isn't supported.

I will say though, given that you don't care about being a "true JS" engine, many things in JS would be trivial just treat as "syntactic sugar" for things that already exist in Java/Karate if you are fine with breaking "true JS" semantics. Async/await would map trivially to Java concurrency, for example - async could just wrap a JsFunction in ExecutorService.submit(), and await could just call Future.get().

ptrthomas commented 3 months ago

LoC is an absolutely terrible benchmark 99% of the time

I totally agree. but @jbalme you missed the point. which is that when you do this exercise you will get a very clear answer to the question "why don't you fork Nashorn". some things are so easy to say

kind of thing that needs some kind of communication outside of issue trackers and Twitter

this is a very interesting statement and I totally disagree. you seem to be just trying to pick a fight here. the dates for the new engine release have not even been announced and this thread is the first step and to get feedback. as you yourself said, you have the option of remaining on 1.4.1 for years if needed. 1.5.0.RC4 is available also

for the rest of your comments, I will only quote - "talk is cheap. show me the code". I actually encourage you to create the new framework based on cucumber-jvm and graal that you alluded to in your first comment. we can all learn from it.

jbalme commented 3 months ago

which is that when you do this exercise you will get a very clear answer to the question "why don't you fork Nashorn". some things are so easy to say

I've already looked at Nashorn and karate-js. Nashorn looks much easier to maintain - if - you don't care about new language features or sandboxing-related CVEs - which you don't seem to.

you seem to be just trying to pick a fight here.

Mate, you're the one trying to pick a fight.

you have the option of remaining on 1.4.1 for years if needed

I mean, creating an internal fork of 1.4.1 definitely seems like what I'll be doing in the short term in order to keep our existing tests running while fixing some of the problems I have with karate-ui - but I'm trying to avoid maintaining a framework that doesn't (won't any longer) exist outside of our org long term so we'd have to either migrate to 1.5.x or off Karate.

I actually encourage you to create the new framework based on cucumber-jvm and graal that you alluded to in your first comment. we can all learn from it.

It wouldn't be a full framework, it would just be a handful of pre-baked step definitions that match some of karate's commands so we could migrate to Cucumber + RestAssured while keeping a decent number of our test cases that only do basic HTTP assertions.

Things like reading rows from a csv or yaml in a Scenario Outline, and doing nested feature calls are really nice and I wouldn't be able to implement them on top of Cucumber without more effort than it would be worth but alas.

ptrthomas commented 3 months ago

Nashorn looks much easier to maintain

wow , let's just agree to disagree here. peace

joelpramos commented 3 months ago

I strongly feel that karate-js being a subset of "real JS" is a very VERY good thing.

Then I would recommend rebranding it away from the name JS so people don't get the wrong idea, and explicitly documenting the new DSL giving a list of what is and isn't supported.

That's a pretty good idea from an optics perspective. That way you are not breaking JS, you are making an explicit decision of empowering Karate test scripters to maximize the use of the framework.

Why not karate-scripting? The language to allow extensions and powerful functions that also boxes itself within the realm of what is not possible in karate...

from a release standpoint you can call it as compliant with JS whichever version makes sense and explicitly call out how decision is to enforce cleaner testing scripting to ensure teams can maximize the powerful features of Karate etc. ... and backlog the capability for pluggable JS for future release (for hopefully get a community member to help).

reality is most folks won't follow the rabbit hole of why graal vs why not

ptrthomas commented 3 months ago

That's a pretty good idea from an optics perspective

personally I feel karate-js is a valid name, because it is valid JS and we actually use the official ECMA script test-suite. if we called it something else - guess what - people would then complain that they need to learn a new language, right ?

the release of the karate-js project is because we feel there is great value in a new JS implementation, and if people want - they can add the missing features. but - it is indeed possible that no one will care, and karate-js could end up as a nameless scripting engine that is embedded within karate and used by nobody else

which is totally fine and then no one would complain any more - and that would be end of this discussion !

sskumargithub commented 3 months ago

@ptrthomas is there any documentation that helps in using karate-js instead of Graal. We are currently facing the below error when the code reads a Java file in multi thread application.

ptrthomas commented 3 months ago

@sskumargithub this is most likely because your classpath is not set up correctly. I think you should follow this process: https://github.com/karatelabs/karate/wiki/How-to-Submit-an-Issue

sskumargithub commented 3 months ago

@ptrthomas I will post sample project shortly but out of 4 test cases running in parallel 1 is passing but remaining 3 are failing. We use the same scenario karate script as we pass the test data dynamically.

Also, the code works fine in local but it's failing when deployed in AWS with the above error.