-
Notifications
You must be signed in to change notification settings - Fork 15
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
#969: Use new CTPG parser in java vm #455
Merged
tomuben
merged 43 commits into
master
from
refactoring/969_use_new_ctpg_parser_in_java_vm
Oct 15, 2024
Merged
Changes from 1 commit
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
77e9955
#969: Use new CTPG parser in java vm
tomuben 780ff35
Added CTPG specific tests
tomuben 6739678
Findings from review
tomuben 3c66a0b
Limit max recursion for script importer and added a test.
tomuben ba28b85
Added valgrind/asan tests
tomuben fa1ccb9
Fixed GH Workflow
tomuben dfdda46
Fixed GH Workflow
tomuben 79553b2
Fixed GH Workflow
tomuben b3c1c7c
Fixed GH Workflow
tomuben 25e21e9
Fixed GH Workflow
tomuben 0ef2b94
Fixed GH Workflow
tomuben 9be6303
Fixed GH Workflow
tomuben ee31d02
Fixed memory allocation/deallocation issue reported by ASAN in tests
tomuben f9663d1
Upload test logs only in error case
tomuben c53ce9e
Deactivate slow tests under valgrind
tomuben 9dba9d2
Inject a memory problem....let's see which checks find the problem....
tomuben cd61bc0
Removed memory corruption
tomuben a145b98
Added bazel config for valgrind
tomuben a01b4fd
Removed (unnecessary) timeout for asan test
tomuben 4607321
Added builder for JavaVM
tomuben 3be1631
Added description to ctpg_script_importer_test.cc
tomuben 60915e5
Optimization in script_option_lines_ctpg.cc
tomuben 9dd5a31
Added performance tests
tomuben 85ca346
Activated valgrind memory leak check
tomuben 56c94fc
Install R for test_package_management_scripts.yaml
tomuben 967c71f
Added comment in parser_ctpg_script_importer.cc
tomuben 8dc5834
Move SwigFactory creation from udf client into JavaContainerBuilder c…
tomuben f1b1940
Fixed JavaContainerBuilder::useCtpgParser
tomuben a846279
Added virtual destructor to SwigFactory.
tomuben ef099fd
Fixed compiler errors
tomuben e63355c
Added a bazl build check
tomuben 5bd6219
Fixed formatting in check_bazel_tests.yml
tomuben 66afcb0
Fixed formatting in check_bazel_tests.yml
tomuben f6ebc7e
Removed Java from bazel build (dependencies not installed)
tomuben 8f570d6
Added Python env variables in check_bazel_tests.yml
tomuben 89f29f2
fixed Python env variables in check_bazel_tests.yml
tomuben c6ca998
Added protobuf compiler to build job in check_bazel_tests.yml
tomuben 53c9606
Created installUdfClientDeps.sh
tomuben d69208b
Run installUdfClientDeps.sh as sudo
tomuben f09a77d
Update exaudfclient/base/javacontainer/script_options/parser_ctpg_scr…
tomuben 4287b0e
Don't use installUdfClientDeps.sh for bazel test
tomuben e39d4c7
Fixed installation of bazel in check_bazel_tests.yml
tomuben c812821
Fixed execution of bazel tests in check_bazel_tests.yml
tomuben File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
108 changes: 108 additions & 0 deletions
108
exaudfclient/base/javacontainer/test/cpp/javacontainer_ctpg_test.cc
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
|
||
#include "include/gtest/gtest.h" | ||
#include "gmock/gmock.h" | ||
#include "base/javacontainer/test/cpp/javavm_test.h" | ||
#include "base/javacontainer/test/cpp/swig_factory_test.h" | ||
#include <string.h> | ||
|
||
class JavaContainerEscapeSequenceTest : public ::testing::TestWithParam<std::pair<std::string, std::string>> {}; | ||
|
||
TEST_P(JavaContainerEscapeSequenceTest, quoted_jvm_option) { | ||
const std::pair<std::string, std::string> option_value = GetParam(); | ||
const std::string script_code = | ||
"%jvmoption " + option_value.first + ";\n\n" | ||
"class JVMOPTION_TEST_WITH_SPACE {\n" | ||
"static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" | ||
" ctx.emit(\"Success!\");\n" | ||
" }\n" | ||
"}\n"; | ||
JavaVMTest vm(script_code); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJavaPath, "/exaudf/base/javacontainer"); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_localClasspath, "/tmp"); | ||
const std::string expected_script_code = | ||
"package com.exasol;\r\n\n\n" | ||
"class JVMOPTION_TEST_WITH_SPACE {\n" | ||
"static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" | ||
"\tctx.emit(\"Success!\");\n" | ||
" }\n}\n"; | ||
EXPECT_EQ(expected_script_code, vm.getJavaVMInternalStatus().m_scriptCode); | ||
EXPECT_EQ("/exaudf/base/javacontainer/exaudf_deploy.jar", vm.getJavaVMInternalStatus().m_exaJarPath); | ||
EXPECT_EQ("/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar", vm.getJavaVMInternalStatus().m_classpath); | ||
EXPECT_TRUE(vm.getJavaVMInternalStatus().m_needsCompilation); | ||
|
||
const std::vector<std::string> expectedJVMOptions = { option_value.second, "-Xms128m", "-Xmx128m", "-Xss512k", | ||
"-XX:ErrorFile=/tmp/hs_err_pid%p.log", | ||
"-Djava.class.path=/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar", | ||
"-XX:+UseSerialGC" }; | ||
EXPECT_EQ(expectedJVMOptions, vm.getJavaVMInternalStatus().m_jvmOptions); | ||
} | ||
|
||
const std::vector<std::pair<std::string, std::string>> escape_sequences = | ||
{ | ||
std::make_pair("-Dhttp.agent=ABC\\nDEF", "-Dhttp.agent=ABC\nDEF"), | ||
std::make_pair("-Dhttp.agent=ABC\\rDEF", "-Dhttp.agent=ABC\rDEF"), | ||
std::make_pair("-Dhttp.agent=ABC\\;DEF", "-Dhttp.agent=ABC;DEF"), | ||
std::make_pair("-Dhttp.agent=ABC\\aDEF", "-Dhttp.agent=ABC\\aDEF"), //any other escape sequence must stay as is | ||
std::make_pair("\\n-Dhttp.agent=ABCDEF", "\n-Dhttp.agent=ABCDEF"), | ||
std::make_pair("\\r-Dhttp.agent=ABCDEF", "\r-Dhttp.agent=ABCDEF"), | ||
std::make_pair("\\;-Dhttp.agent=ABCDEF", ";-Dhttp.agent=ABCDEF"), | ||
std::make_pair("-Dhttp.agent=ABCDEF\\n", "-Dhttp.agent=ABCDEF\n"), | ||
std::make_pair("-Dhttp.agent=ABCDEF\\r", "-Dhttp.agent=ABCDEF\r"), | ||
std::make_pair("-Dhttp.agent=ABCDEF\\;", "-Dhttp.agent=ABCDEF;"), | ||
std::make_pair("\\ -Dhttp.agent=ABCDEF", "-Dhttp.agent=ABCDEF"), | ||
std::make_pair("\\t-Dhttp.agent=ABCDEF", "-Dhttp.agent=ABCDEF"), | ||
std::make_pair("\\f-Dhttp.agent=ABCDEF", "-Dhttp.agent=ABCDEF"), | ||
std::make_pair("\\v-Dhttp.agent=ABCDEF", "-Dhttp.agent=ABCDEF") | ||
}; | ||
|
||
INSTANTIATE_TEST_SUITE_P( | ||
JavaContainer, | ||
JavaContainerEscapeSequenceTest, | ||
::testing::ValuesIn(escape_sequences) | ||
); | ||
|
||
TEST(JavaContainer, import_script_with_escaped_options) { | ||
const std::string script_code = | ||
"%import other_script;\n\n" | ||
"%jvmoption -Dsomeoption=\"ABC\";\n\n" | ||
"%scriptclass com.exasol.udf_profiling.UdfProfiler;\n" | ||
"%jar base/javacontainer/test/test.jar;" | ||
"class JVMOPTION_TEST {\n" | ||
"static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" | ||
" ctx.emit(\"Success!\");\n" | ||
" }\n" | ||
"}\n"; | ||
SwigFactoryTestImpl swigFactory; | ||
|
||
const std::string other_script_code = | ||
"%jvmoption -Dsomeotheroption=\"DE\\nF\";\n\n" | ||
"%jar base/javacontainer/test/other_test.jar;" | ||
"class OtherClass {\n" | ||
"static void doSomething() {\n\n" | ||
" }\n" | ||
"}\n"; | ||
swigFactory.addModule("other_script", other_script_code); | ||
JavaVMTest vm(script_code, swigFactory); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJavaPath, "/exaudf/base/javacontainer"); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_localClasspath, "/tmp"); | ||
const std::string expected_script_code = | ||
"package com.exasol;\r\n\n\n" | ||
"class OtherClass {\n" | ||
"static void doSomething() {\n\n" | ||
" }\n" | ||
"}\n\n\n\n\n\n" | ||
"class JVMOPTION_TEST {\n" | ||
"static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {\n\n" | ||
"\tctx.emit(\"Success!\");\n" | ||
" }\n}\n"; | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_scriptCode, expected_script_code); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJarPath, "/exaudf/base/javacontainer/exaudf_deploy.jar"); | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/other_test.jar:base/javacontainer/test/test.jar"); | ||
EXPECT_TRUE(vm.getJavaVMInternalStatus().m_needsCompilation); | ||
const std::vector<std::string> expectedJVMOptions = { "-Dexasol.scriptclass=com.exasol.udf_profiling.UdfProfiler", | ||
"-Dsomeotheroption=\"DE\nF\"", "-Dsomeoption=\"ABC\"", "-Xms128m", "-Xmx128m", "-Xss512k", | ||
"-XX:ErrorFile=/tmp/hs_err_pid%p.log", | ||
"-Djava.class.path=/tmp:/exaudf/base/javacontainer/exaudf_deploy.jar:base/javacontainer/test/other_test.jar:base/javacontainer/test/test.jar", | ||
"-XX:+UseSerialGC" }; | ||
EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions); | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Do we have a test for escaped whitespace in the middle of a option, because this was one of the cases that was as bug reported
-Dhttp.agent=ABC\ DEF;
This is less of an option line parser issue then an issue of the value handling of the Jvm options splitter
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.
This bug is not fixed yet.
My plan was to fix exasol/script-languages-release#878 in another PR, in order to keep a clean git history.
In the scope of #878 I will extend the test here.