#652: Started unit tests for script option lines parser

.github/workflows/check_bazel_tests.yml

@@ -25,8 +25,14 @@ jobs:
       run: |
         sudo apt-get update
         sudo apt-get install -y openjdk-11-jdk libzmq3-dev
-    - name: Test
+    - name: Java Tests
       run: |
         export USE_BAZEL_VERSION=6.4.0
         bazel test //javacontainer/test/...
       working-directory: ./exaudfclient/base
+    - name: ExaudfLib Tests
+      run: |
+        export USE_BAZEL_VERSION=6.4.0
+        bazel test //exaudflib/test/...
+      working-directory: ./exaudfclient/base
+

exaudfclient/base/exaudflib/test/BUILD

@@ -0,0 +1,9 @@
+cc_test(
+    name = "exaudflib-test",
+    srcs = ["script_data_transfer_objects_test.cpp", "script_option_lines_test.cpp"],
+    deps = [
+        "//exaudflib:script_data_transfer_objects",
+        "//exaudflib:scriptoptionlines",
+        "@googletest//:gtest_main",
+    ],
+)

exaudfclient/base/exaudflib/test/script_data_transfer_objects_test.cpp

@@ -1,4 +1,4 @@
-#include <engine/exscript/script_data_transfer_objects.h>
+#include "exaudflib/swig/script_data_transfer_objects.h"
 #include <gtest/gtest.h>
 
 using namespace ExecutionGraph;
@@ -17,8 +17,8 @@ class ImportSpecificationTest : public ::testing::Test {
 TEST_F(ImportSpecificationTest, is_subselect)
 {
     ImportSpecification is1(true);
-    EXPECT_TRUE(is1.isSubselect());
     ImportSpecification is2(false);
+    EXPECT_TRUE(is1.isSubselect());
     EXPECT_FALSE(is2.isSubselect());
 }
 
@@ -116,14 +116,14 @@ TEST_F(ImportSpecificationTest, column_names_and_types)
 
     EXPECT_TRUE(is1.hasSubselectColumnSpecification());
     EXPECT_TRUE(is1.hasConsistentColumns());
-    EXPECT_TRUE(is1.getSubselectColumnNames().size() == 3);
-    EXPECT_TRUE(is1.getSubselectColumnTypes().size() == 3);
-    EXPECT_TRUE(is1.getSubselectColumnNames()[0] == "col1");
-    EXPECT_TRUE(is1.getSubselectColumnNames()[1] == "col2");
-    EXPECT_TRUE(is1.getSubselectColumnNames()[2] == "col3");
-    EXPECT_TRUE(is1.getSubselectColumnTypes()[0] == "type1");
-    EXPECT_TRUE(is1.getSubselectColumnTypes()[1] == "type2");
-    EXPECT_TRUE(is1.getSubselectColumnTypes()[2] == "type3");
+    EXPECT_EQ(is1.getSubselectColumnNames().size(), 3);
+    EXPECT_EQ(is1.getSubselectColumnTypes().size(), 3);
+    EXPECT_EQ(is1.getSubselectColumnNames()[0], "col1");
+    EXPECT_EQ(is1.getSubselectColumnNames()[1], "col2");
+    EXPECT_EQ(is1.getSubselectColumnNames()[2], "col3");
+    EXPECT_EQ(is1.getSubselectColumnTypes()[0], "type1");
+    EXPECT_EQ(is1.getSubselectColumnTypes()[1], "type2");
+    EXPECT_EQ(is1.getSubselectColumnTypes()[2], "type3");
 }
 
 TEST_F(ImportSpecificationTest, connection_name_or_details)
@@ -175,14 +175,14 @@ TEST_F(ImportSpecificationTest, connection_info)
 {
     ImportSpecification is1(false);
     is1.setConnectionName("some_connection");
-    EXPECT_TRUE(is1.getConnectionName() == "some_connection");
+    EXPECT_EQ(is1.getConnectionName(), "some_connection");
 
     ImportSpecification is2(false);
     is2.setConnectionInformation(ConnectionInformation("some_address","some_user","some_password"));
-    EXPECT_TRUE(is2.getConnectionInformation().getAddress() == "some_address");
-    EXPECT_TRUE(is2.getConnectionInformation().getUser() == "some_user");
-    EXPECT_TRUE(is2.getConnectionInformation().getPassword() == "some_password");
-    EXPECT_TRUE(is2.getConnectionInformation().getKind() == "password");
+    EXPECT_EQ(is2.getConnectionInformation().getAddress(), "some_address");
+    EXPECT_EQ(is2.getConnectionInformation().getUser(), "some_user");
+    EXPECT_EQ(is2.getConnectionInformation().getPassword(), "some_password");
+    EXPECT_EQ(is2.getConnectionInformation().getKind(), "password");
 }
 
 
@@ -193,15 +193,15 @@ TEST_F(ImportSpecificationTest, parameters)
     is1.addParameter("key1","value1");
     EXPECT_TRUE(is1.hasParameters());
     std::map<std::string, std::string>::const_iterator ps1 = is1.getParameters().begin();
-    EXPECT_TRUE(ps1->first == "key1");
-    EXPECT_TRUE(ps1->second == "value1");
+    EXPECT_EQ(ps1->first, "key1");
+    EXPECT_EQ(ps1->second, "value1");
 
     is1.addParameter("key2","value2");
     EXPECT_TRUE(is1.hasParameters());
     std::map<std::string, std::string>::const_iterator ps2 = is1.getParameters().begin();
     ++ps2;
-    EXPECT_TRUE(ps2->first == "key2");
-    EXPECT_TRUE(ps2->second == "value2");
+    EXPECT_EQ(ps2->first, "key2");
+    EXPECT_EQ(ps2->second, "value2");
 }
 
 //
@@ -221,46 +221,40 @@ class ConnectionInformationTest : public ::testing::Test {
 TEST_F(ConnectionInformationTest, address_user_password_construction)
 {
     ConnectionInformation ci("some_address","some_user","some_password");
-    EXPECT_TRUE(ci.getKind() == "password");
-    EXPECT_TRUE(ci.getAddress() == "some_address");
-    EXPECT_TRUE(ci.getUser() == "some_user");
-    EXPECT_TRUE(ci.getPassword() == "some_password");
+    EXPECT_EQ(ci.getKind(), "password");
+    EXPECT_EQ(ci.getAddress(), "some_address");
+    EXPECT_EQ(ci.getUser(), "some_user");
+    EXPECT_EQ(ci.getPassword(), "some_password");
     EXPECT_FALSE(ci.hasData());
 }
 
 TEST_F(ConnectionInformationTest, kind_address_user_password_construction)
 {
     ConnectionInformation ci("some_kind","some_address","some_user","some_password");
-    EXPECT_TRUE(ci.getKind() == "some_kind");
-    EXPECT_TRUE(ci.getAddress() == "some_address");
-    EXPECT_TRUE(ci.getUser() == "some_user");
-    EXPECT_TRUE(ci.getPassword() == "some_password");
+    EXPECT_EQ(ci.getKind(), "some_kind");
+    EXPECT_EQ(ci.getAddress(), "some_address");
+    EXPECT_EQ(ci.getUser(), "some_user");
+    EXPECT_EQ(ci.getPassword(), "some_password");
     EXPECT_FALSE(ci.hasData());
 }
 
 TEST_F(ConnectionInformationTest, empty_construction)
 {
     ConnectionInformation ci;
-    EXPECT_TRUE(ci.getKind() == "");
-    EXPECT_TRUE(ci.getAddress() == "");
-    EXPECT_TRUE(ci.getUser() == "");
-    EXPECT_TRUE(ci.getPassword() == "");
+    EXPECT_TRUE(ci.getKind().empty());
+    EXPECT_TRUE(ci.getAddress().empty());
+    EXPECT_TRUE(ci.getUser().empty());
+    EXPECT_TRUE(ci.getPassword().empty());
     EXPECT_TRUE(ci.hasData());
 }
 
 TEST_F(ConnectionInformationTest, copy)
 {
     ConnectionInformation ca("a","b","c","d");
     ConnectionInformation cb(ca);
-    EXPECT_TRUE(ca.getKind() == cb.getKind());
-    EXPECT_TRUE(ca.getAddress() == cb.getAddress());
-    EXPECT_TRUE(ca.getUser() == cb.getUser());
-    EXPECT_TRUE(ca.getPassword() == cb.getPassword());
-    EXPECT_TRUE(ca.hasData() == cb.hasData());
-}
-
-
-int main(int argc, char **argv) {
-    ::testing::InitGoogleTest(&argc, argv);
-    return RUN_ALL_TESTS();
+    EXPECT_EQ(ca.getKind(), cb.getKind());
+    EXPECT_EQ(ca.getAddress(), cb.getAddress());
+    EXPECT_EQ(ca.getUser(), cb.getUser());
+    EXPECT_EQ(ca.getPassword(), cb.getPassword());
+    EXPECT_EQ(ca.hasData(), cb.hasData());
 }

exaudfclient/base/exaudflib/test/script_option_lines_test.cpp

@@ -0,0 +1,185 @@
+#include "exaudflib/vm/scriptoptionlines.h"
+#include <gtest/gtest.h>
+#include <string>
+#include <exception>
+
+const std::string whitespace = " \t\f\v";
+const std::string lineEnd = ";";
+
+class TestException : public std::runtime_error {
+    using std::runtime_error::runtime_error;
+};
+
+void throwException(const char* ex) {
+    throw TestException(std::string(ex));
+}
+
+using namespace ExecutionGraph;
+
+
+class ScriptOptionLinesWhitespaceTest : public ::testing::TestWithParam<std::tuple<std::string, std::string, std::string, std::string, std::string>> {};
+
+TEST_P(ScriptOptionLinesWhitespaceTest, WhitespaceExtractOptionLineTest) {
+    size_t pos;
+    const std::string prefix = std::get<0>(GetParam());
+    const std::string suffix = std::get<1>(GetParam());
+    const std::string option = std::get<2>(GetParam());
+    const std::string value = std::get<3>(GetParam());
+    const std::string payload =  std::get<4>(GetParam());
+    std::string code = prefix + option + value + lineEnd + suffix + "\n" + payload;
+    const std::string res = extractOptionLine(code, option, whitespace, lineEnd, pos, throwException);
+    EXPECT_EQ(res, value);
+    EXPECT_EQ(code, prefix + suffix + "\n" + payload);
+}
+
+std::vector<std::string> white_space_strings = {"", " ", "\t", "\f", "\v", "\n", " \t", "\t ", "\t\f", "\f\t", "\f ", " \f", "\t\v", "\v\t", "\v ", " \v", "\f\v", "\v\f", "  \t", " \t "};
+std::vector<std::string> keywords = {"%import", "jvmoption", "%scriptclass", "%jar", "%env"};
+std::vector<std::string> values = {"something", "com.mycompany.MyScriptClass", "LD_LIBRARY_PATH=/nvdriver", "-Xms128m -Xmx1024m -Xss512k", "/buckets/bfsdefault/default/my_code.jar"};
+std::vector<std::string> payloads = {"anything", "\n\ndef my_func:\n\tpass", "class MyJava\n public static void Main() {\n};\n"};
+
+INSTANTIATE_TEST_SUITE_P(
+    ScriptOptionLines,
+    ScriptOptionLinesWhitespaceTest,
+    ::testing::Combine(::testing::ValuesIn(white_space_strings),
+                       ::testing::ValuesIn(white_space_strings),
+                       ::testing::ValuesIn(keywords),
+                       ::testing::ValuesIn(values),
+                       ::testing::ValuesIn(payloads)
+    )
+);
+
+TEST(ScriptOptionLinesTest, ignore_anything_other_than_whitepsace) {
+    size_t pos;
+    std::string code =
+        "abc %option myoption;\n"
+        "\nmycode";
+    const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException);
+    EXPECT_TRUE(res.empty());
+}
+
+TEST(ScriptOptionLinesTest, need_line_end_character) {
+    size_t pos;
+    std::string code =
+        "%option myoption\n"
+        "\nmycode";
+   EXPECT_THROW({
+        const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException);
+    }, TestException );
+}
+
+TEST(ScriptOptionLinesTest, only_finds_the_first_option_same_key) {
+    size_t pos;
+    std::string code =
+        "%option myoption; %option mysecondoption;\n"
+        "\nmycode";
+    const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException);
+    const std::string expected_resulting_code =
+        " %option mysecondoption;\n"
+        "\nmycode";
+
+    EXPECT_EQ(res, "myoption");
+    EXPECT_EQ(code, expected_resulting_code);
+}
+
+TEST(ScriptOptionLinesTest, only_finds_the_first_option_different_key) {
+    size_t pos;
+    std::string code =
+        "%option myoption; %otheroption mysecondoption;\n"
+        "\nmycode";
+    const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException);
+    const std::string expected_resulting_code =
+        " %otheroption mysecondoption;\n"
+        "\nmycode";
+
+    EXPECT_EQ(res, "myoption");
+    EXPECT_EQ(code, expected_resulting_code);
+}
+
+class ScriptOptionLinesInvalidOptionTest : public ::testing::TestWithParam<std::string> {};
+
+
+TEST_P(ScriptOptionLinesInvalidOptionTest, value_is_mandatory) {
+    size_t pos;
+    const std::string invalid_option = GetParam();
+    std::string code = invalid_option + "\nsomething";
+    EXPECT_THROW({
+     const std::string res = extractOptionLine(code, "%option", whitespace, lineEnd, pos, throwException);
+    }, TestException );
+}
+
+std::vector<std::string> invalid_options = {"%option ;", "%option \n", "\n%option\n;", "%option\nvalue;"};
+
+INSTANTIATE_TEST_SUITE_P(
+    ScriptOptionLines,
+    ScriptOptionLinesInvalidOptionTest,
+    ::testing::ValuesIn(invalid_options)
+);
+
+TEST(ScriptOptionLinesTest, ignores_any_other_option) {
+    size_t pos;
+    const std::string original_code =
+        "%option myoption; %option mysecondoption;\n"
+        "\nmycode";
+    std::string code = original_code;
+    const std::string res = extractOptionLine(code, "%mythirdoption", whitespace, lineEnd, pos, throwException);
+    EXPECT_TRUE(res.empty());
+    EXPECT_EQ(code, original_code);
+}
+
+
+TEST(ScriptOptionLinesTest, test_all_in_one_line_does_second_option_does_not_work) {
+    /**
+    Verify the wrong behavior and assumptions as described in https://github.com/exasol/script-languages-release/issues/652.
+    Here we call `extractOptionLine()` with the keys in the order of the new implementation (first for key 'jvmoption').
+    This is supposed to not work.
+    */
+    size_t pos;
+    const std::string original_code = "%jar /buckets/bucketfs1/jars/exajdbc.jar; %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;";
+    std::string code = original_code;
+    const std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException);
+    EXPECT_TRUE(res.empty());
+    EXPECT_EQ(code, original_code);
+}
+
+TEST(ScriptOptionLinesTest, test_all_in_one_line_does_first_option_does_work) {
+    /**
+    Verify the wrong behavior and assumptions as described in https://github.com/exasol/script-languages-release/issues/652.
+    Here we call `extractOptionLine()` with the keys in the order of the old implementation (first for key '%jar', then for key 'jvmoption').
+    This is supposed to work.
+    */
+    size_t pos;
+    const std::string original_code = "%jar /buckets/bucketfs1/jars/exajdbc.jar; %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;";
+    std::string code = original_code;
+    std::string res = extractOptionLine(code, "%jar", whitespace, lineEnd, pos, throwException);
+    EXPECT_EQ(res, "/buckets/bucketfs1/jars/exajdbc.jar");
+    EXPECT_EQ(code, " %jvmoption -Xms4m; class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;");
+    res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException);
+    EXPECT_EQ(code, "  class JAVA_UDF_3 {static void run(ExaMetadata exa, ExaIterator ctx) throws Exception {String host_name = ctx.getString(\"col1\");}}\n/\n;");
+}
+
+TEST(ScriptOptionLinesTest, test_values_must_not_contain_spaces) {
+    /**
+    Verify the wrong behavior and assumptions as described in https://github.com/exasol/script-languages-release/issues/878
+    The parser is actually correct, but the client code incorrectly parses the result (see javacontainer_test.cc - quoted_jvm_option)
+    */
+    size_t pos;
+    const std::string original_code =
+        "%jvmoption -Dhttp.agent=\"ABC DEF\";\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";
+    std::string code = original_code;
+    std::string res = extractOptionLine(code, "%jvmoption", whitespace, lineEnd, pos, throwException);
+    const std::string expected_result_code =
+        "\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";
+    EXPECT_EQ(res, "-Dhttp.agent=\"ABC DEF\"");
+    EXPECT_EQ(code, expected_result_code);
+}
+

exaudfclient/base/javacontainer/test/cpp/javacontainer_test.cc

@@ -86,4 +86,36 @@ TEST(JavaContainer, combined_inline_jar) {
 }
 
 
-
+TEST(JavaContainer, quoted_jvm_option) {
+    const std::string script_code =
+        "%jvmoption -Dhttp.agent=\"ABC DEF\";\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/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(vm.getJavaVMInternalStatus().m_scriptCode, expected_script_code);
+    EXPECT_EQ(vm.getJavaVMInternalStatus().m_exaJarPath, "/exaudf/javacontainer/libexaudf.jar");
+    EXPECT_EQ(vm.getJavaVMInternalStatus().m_classpath, "/tmp:/exaudf/javacontainer/libexaudf.jar");
+    const std::vector<std::string> expectedJarPaths = {};
+    EXPECT_EQ(vm.getJavaVMInternalStatus().m_jarPaths, expectedJarPaths);
+    EXPECT_TRUE(vm.getJavaVMInternalStatus().m_needsCompilation);
+    /*
+     * Note: The option "DEF" is wrong and causes UDF's to crash!
+     *       The correct option would be '-Dhttp.agent=\"ABC DEF\"'
+     */
+    const std::vector<std::string> expectedJVMOptions = {   "-Dhttp.agent=\"ABC", "DEF\"", "-Xms128m", "-Xmx128m", "-Xss512k",
+                                                            "-XX:ErrorFile=/tmp/hs_err_pid%p.log",
+                                                            "-Djava.class.path=/tmp:/exaudf/javacontainer/libexaudf.jar",
+                                                            "-XX:+UseSerialGC" };
+    EXPECT_EQ(vm.getJavaVMInternalStatus().m_jvmOptions, expectedJVMOptions);
+}