-
Notifications
You must be signed in to change notification settings - Fork 69
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
Adds support for scriptable datasets via Java scripting api (JSR 233) #47
base: master
Are you sure you want to change the base?
Conversation
Awesome stuff! Thanks a lot for your contribution. Let me just get back
|
INSERT INTO useraccount (id, firstname, lastname, username, password) VALUES (1, 'John', 'Smith', 'doovde', 'password'); | ||
INSERT INTO useraccount (id, firstname, lastname, username, password) VALUES (2, 'Clark', 'Kent', 'superman', 'kryptonite'); | ||
INSERT INTO useraccount (id, firstname, lastname, username, password) VALUES (1, 'John', 'Smith', 'doovde', 'password'); | ||
INSERT INTO useraccount (id, firstname, lastname, username, password) VALUES (2, 'Clark', 'Kent', 'superman', 'kryptonite'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it line-ending change b/c of windows? If so I would suggest to adjust your IDE and re-format to linux line ending :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, can be because I commited from both envs, it shouldn't be on the PR.
- removes windows special characters;
Hi @bartoszmajsak, I think the last commit fixes above issues. |
<dependency> | ||
<groupId>org.codehaus.groovy</groupId> | ||
<artifactId>groovy-all</artifactId> | ||
<version>2.4.6</version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move the version to the property like for other artifacts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure
Great, thanks! I shared just few small things but we are close to bringing it to the upstream. Thanks a lot for your contribution. I will work on this tonight. Could you also add some small note about this functionality in |
- adds ScriptableDataSetEngineException and ScriptableDataSetEvalutationException.
Hi @bartoszmajsak, added readme and fixed exception handling. Let me know if there is anything else to refactory. |
Overall it's awesome contribution. And it's been very long on my list. The only thing we have left is to make sure that we somehow support (or not) those containers which are currently causing the issues, as you highlighted in the initial comment. Before that we cannot really fully announce that we have such a functionality. I believe we have to investigate if it is fixable or not. In both cases we should it documented, but also make problematic tests not run in those environments. I will take it over from now on. Once again, many thanks! |
Yea, fully agreed. Here is what I've tested: On wildfly for example it works for 8.2.0/8.2.1(jdk7 and 8) but fails on 8.1.0 with message: ScriptEngineManager providers.next(): javax.script.ScriptEngineFactory: Provider jdk.nashorn.api.scripting.NashornScriptEngineFactory not found On jboss 7.0.2 it didn't worked at all with error mentioned in first message here. I was not able to test on other containers like glassfish and tomee, the maven profiles should work? |
They should. Or at least used to :) But I will need to update those too, so I will check how does it behave in other envs we plan to support. |
refs #18
There is a limitation for the Javascript engine which is bundled with the jdk. It is working on some cases (Wildfly jdk 7) but failing on others (jboss as) with the following error:
The error is thrown on a variety of cases like JDK version (on jdk8 engine was changed from rhino to Nashorn) and server version and is pretty uninstallable.
Groovy engine seems to be more stable and work on all cases I've tested. Its dependency (groovy-all) is being bundle within arquillian persistence extension.
The implementation is parsing the script engine from dataset and, in theory, any script engine can be used.
Also by default this feature is not enabled and needs to be settled in arquillian.xml.
What do you think?