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

replace graal with karate-js engine #2546

Open
ptrthomas opened this issue Apr 9, 2024 · 32 comments
Open

replace graal with karate-js engine #2546

ptrthomas opened this issue Apr 9, 2024 · 32 comments
Assignees
Milestone

Comments

@ptrthomas
Copy link
Member

ptrthomas commented Apr 9, 2024

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

this will give us the following advantages:

  • reduce JAR size by at least 30 MB if not more
  • no limits to re-using JS code, even by call-once and call-single
  • no more depending on 3rd party project (fewer CVEs and less risk in case that project changes direction)
  • possibly improve performance
  • foundation for extending karate in the future (e.g. an alternative to cucumber for those who want BDD)
  • support debugging of even JS blocks in IDE plugins
@ptrthomas
Copy link
Member Author

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: ba931c8

@maxandersen
Copy link
Contributor

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
Copy link
Member Author

@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
Copy link

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
Copy link

woess commented Apr 22, 2024

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
Copy link

jbalme commented May 23, 2024

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
Copy link
Member Author

@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
Copy link

jbalme commented May 23, 2024

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
Copy link
Member Author

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

@ivangsa
Copy link
Contributor

ivangsa commented May 23, 2024

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
Copy link
Member Author

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
Copy link

ericdriggs commented May 23, 2024

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

  • poor performance due to excessive locking burden
  • extreme difficulty in passing data across contexts.
    The readme covers this pretty well.

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
Copy link

jbalme commented May 24, 2024

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
Copy link

jbalme commented May 24, 2024

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
Copy link
Member Author

@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
Copy link

jbalme commented May 24, 2024

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
Copy link

jbalme commented May 24, 2024

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
Copy link
Member Author

ptrthomas commented May 24, 2024

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
Copy link

@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
Copy link
Member Author

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
Copy link
Contributor

joelpramos commented May 24, 2024

@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
Copy link
Member Author

@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
Copy link

ericdriggs commented May 24, 2024

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
Copy link

jbalme commented May 25, 2024

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
Copy link
Member Author

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
Copy link

jbalme commented May 25, 2024

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
Copy link
Member Author

Nashorn looks much easier to maintain

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

@joelpramos
Copy link
Contributor

joelpramos commented May 27, 2024

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
Copy link
Member Author

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
Copy link

@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.

  • def XmlToJsonConv = Java.type('com.util.XmlConvt')
    js failed:

01: Java.type('com.util.XmlConvt')
<<<<
org.graalvm.polyglot.PolyglotException: TypeError: Access to host class com.util.XmlConvt is not allowed or does not exist.

  • .:program(Unnamed:1)

@ptrthomas
Copy link
Member Author

@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
Copy link

sskumargithub commented May 31, 2024

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants