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

GROOVY-7697: GroovyScriptEngine.loadScriptByName doesn't support envi… #330

Closed
wants to merge 1 commit into from

Conversation

andresteingress
Copy link
Contributor

…ronment variable "groovy.ast"

This PR fixes an issue where the "groovy.ast" dump (only god, its author and the ticket author knew this existed) would not be saved when a script was loaded by name via the GSE. In that case, the XStreamUtils instance was given an URI (String) instead of an absolute path and the code there broke.

Regarding error handling in XStreamUtils (as this was also mentioned in the ticket): I did not change anything into that direction as I am not convinced that such a secondary "add-on" feature should in any way influence behaviour in SourceUnit#convert.

@PascalSchumacher
Copy link
Contributor

When I run the build with the changes from the pull request locally (where tests run in parallel), this test always fails:

groovy.util.ProxyGeneratorTest > testUnknownMethodThrowsUnsupportedOperationException FAILED
groovy.lang.MissingMethodException: No signature of method: static groovy.util.ProxyGenerator.instantiateAggregateFromInterface() is applicable fo
r argument types: (java.util.LinkedHashMap, java.lang.Class) values: [[myMethodA:groovy.util.ProxyGeneratorTest$_testUnknownMethodThrowsUnsupportedOpe
rationException_closure16@6cb59bd9], ...]
Possible solutions: instantiateAggregateFromInterface(java.lang.Class), instantiateAggregateFromInterface(java.util.Map, java.lang.Class)

not sure how it could be related.

@andresteingress
Copy link
Contributor Author

Thanks for hinting on that, @PascalSchumacher. I will have a look at the code again once I find some time this week(end).

@PascalSchumacher
Copy link
Contributor

Thanks. I'm not sure this is caused by the changes of this pull request, maybe the additional tests just reveal a thread-safety issue of other tests (which now run in parallel because of the new tests).

jwagenleitner pushed a commit to jwagenleitner/groovy that referenced this pull request Jul 7, 2016
@asfgit asfgit closed this in a75ea4a Jul 7, 2016
@jwagenleitner
Copy link
Contributor

Thanks!

I tested a build with this PR on several different machines and didn't hit the test failure that @PascalSchumacher had.

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