better error messages

added support for capturing stderr when you expect stuff to fail in
order to capture all error messages from the child process.

simplify ServerCmd to always be verbose (print stdin/stdout(stderr)
interactions).

improve ServerCmd to enable explicitly checking the child process exit
code.

fixed test dependency on binary for eval expr and analyze model tests.

4 files changed, 72 insertions, 6 deletions

diff --git a/eval/src/tests/apps/analyze_onnx_model/CMakeLists.txt b/eval/src/tests/apps/analyze_onnx_model/CMakeLists.txt
index 7b70360a622..06241b0add6 100644
--- a/eval/src/tests/apps/analyze_onnx_model/CMakeLists.txt
+++ b/eval/src/tests/apps/analyze_onnx_model/CMakeLists.txt
@@ -4,6 +4,7 @@ vespa_add_executable(eval_analyze_onnx_model_test_app TEST
     analyze_onnx_model_test.cpp
     DEPENDS
     vespaeval
+    AFTER
+    eval_analyze_onnx_model_app
 )
-vespa_add_test(NAME eval_analyze_onnx_model_test_app COMMAND eval_analyze_onnx_model_test_app
-               DEPENDS eval_analyze_onnx_model_test_app eval_analyze_onnx_model_app)
+vespa_add_test(NAME eval_analyze_onnx_model_test_app COMMAND eval_analyze_onnx_model_test_app)
diff --git a/eval/src/tests/apps/analyze_onnx_model/analyze_onnx_model_test.cpp b/eval/src/tests/apps/analyze_onnx_model/analyze_onnx_model_test.cpp
index 58cc7c22358..72ef0346ea3 100644
--- a/eval/src/tests/apps/analyze_onnx_model/analyze_onnx_model_test.cpp
+++ b/eval/src/tests/apps/analyze_onnx_model/analyze_onnx_model_test.cpp
@@ -17,10 +17,12 @@ vespalib::string get_source_dir() {
 }
 vespalib::string source_dir = get_source_dir();
 vespalib::string probe_model = source_dir + "/../../tensor/onnx_wrapper/probe_model.onnx";
+vespalib::string simple_model = source_dir + "/../../tensor/onnx_wrapper/simple.onnx";
+vespalib::string dynamic_model = source_dir + "/../../tensor/onnx_wrapper/dynamic.onnx";
 
 //-----------------------------------------------------------------------------
 
-TEST_F("require that output types can be probed", ServerCmd(probe_cmd, true)) {
+TEST_F("require that output types can be probed", ServerCmd(probe_cmd)) {
     Slime params;
     params.setObject();
     params.get().setString("model", probe_model);
@@ -32,6 +34,65 @@ TEST_F("require that output types can be probed", ServerCmd(probe_cmd, true)) {
     EXPECT_EQUAL(result["outputs"]["out1"].asString().make_string(), vespalib::string("tensor<float>(d0[2],d1[3])"));
     EXPECT_EQUAL(result["outputs"]["out2"].asString().make_string(), vespalib::string("tensor<float>(d0[2],d1[3])"));
     EXPECT_EQUAL(result["outputs"]["out3"].asString().make_string(), vespalib::string("tensor<float>(d0[2],d1[3])"));
+    EXPECT_EQUAL(f1.shutdown(), 0);
+}
+
+//-----------------------------------------------------------------------------
+
+TEST_F("test error: invalid json", ServerCmd(probe_cmd, ServerCmd::capture_stderr_tag())) {
+    auto out = f1.write_then_read_all("this is not valid json...\n");
+    EXPECT_TRUE(out.find("invalid json") < out.size());
+    EXPECT_EQUAL(f1.shutdown(), 3);
+}
+
+TEST_F("test error: missing input type", ServerCmd(probe_cmd, ServerCmd::capture_stderr_tag())) {
+    Slime params;
+    params.setObject();
+    params.get().setString("model", simple_model);
+    params.get().setObject("inputs");
+    auto out = f1.write_then_read_all(params.toString());
+    EXPECT_TRUE(out.find("missing type") < out.size());
+    EXPECT_EQUAL(f1.shutdown(), 3);
+}
+
+TEST_F("test error: invalid input type", ServerCmd(probe_cmd, ServerCmd::capture_stderr_tag())) {
+    Slime params;
+    params.setObject();
+    params.get().setString("model", simple_model);
+    params.get().setObject("inputs");
+    params["inputs"].setString("query_tensor", "bogus type string");
+    params["inputs"].setString("attribute_tensor", "tensor<float>(x[4],y[1])");
+    params["inputs"].setString("bias_tensor", "tensor<float>(x[1],y[1])");
+    auto out = f1.write_then_read_all(params.toString());
+    EXPECT_TRUE(out.find("invalid type") < out.size());
+    EXPECT_EQUAL(f1.shutdown(), 3);
+}
+
+TEST_F("test error: incompatible input type", ServerCmd(probe_cmd, ServerCmd::capture_stderr_tag())) {
+    Slime params;
+    params.setObject();
+    params.get().setString("model", simple_model);
+    params.get().setObject("inputs");
+    params["inputs"].setString("query_tensor", "tensor<float>(x[1],y[5])");
+    params["inputs"].setString("attribute_tensor", "tensor<float>(x[4],y[1])");
+    params["inputs"].setString("bias_tensor", "tensor<float>(x[1],y[1])");
+    auto out = f1.write_then_read_all(params.toString());
+    EXPECT_TRUE(out.find("incompatible type") < out.size());
+    EXPECT_EQUAL(f1.shutdown(), 3);
+}
+
+TEST_F("test error: symbolic size mismatch", ServerCmd(probe_cmd, ServerCmd::capture_stderr_tag())) {
+    Slime params;
+    params.setObject();
+    params.get().setString("model", dynamic_model);
+    params.get().setObject("inputs");
+    params["inputs"].setString("query_tensor", "tensor<float>(x[1],y[4])");
+    params["inputs"].setString("attribute_tensor", "tensor<float>(x[4],y[1])");
+    params["inputs"].setString("bias_tensor", "tensor<float>(x[2],y[1])");
+    auto out = f1.write_then_read_all(params.toString());
+    EXPECT_TRUE(out.find("incompatible type") < out.size());
+    EXPECT_TRUE(out.find("batch=1") < out.size());
+    EXPECT_EQUAL(f1.shutdown(), 3);
 }
 
 //-----------------------------------------------------------------------------
diff --git a/eval/src/tests/apps/eval_expr/CMakeLists.txt b/eval/src/tests/apps/eval_expr/CMakeLists.txt
index e36c5aafe23..c36cf33e2b2 100644
--- a/eval/src/tests/apps/eval_expr/CMakeLists.txt
+++ b/eval/src/tests/apps/eval_expr/CMakeLists.txt
@@ -4,6 +4,7 @@ vespa_add_executable(eval_eval_expr_test_app TEST
     eval_expr_test.cpp
     DEPENDS
     vespaeval
+    AFTER
+    eval_eval_expr_app
 )
-vespa_add_test(NAME eval_eval_expr_test_app COMMAND eval_eval_expr_test_app
-               DEPENDS eval_eval_expr_test_app eval_eval_expr_app)
+vespa_add_test(NAME eval_eval_expr_test_app COMMAND eval_eval_expr_test_app)
diff --git a/eval/src/tests/apps/eval_expr/eval_expr_test.cpp b/eval/src/tests/apps/eval_expr/eval_expr_test.cpp
index 3834ea0e4cc..fd103829618 100644
--- a/eval/src/tests/apps/eval_expr/eval_expr_test.cpp
+++ b/eval/src/tests/apps/eval_expr/eval_expr_test.cpp
@@ -56,7 +56,7 @@ Result::~Result() = default;
 
 struct Server : public ServerCmd {
     TimeBomb time_bomb;
-    Server() : ServerCmd(server_cmd, true), time_bomb(60) {}
+    Server() : ServerCmd(server_cmd), time_bomb(60) {}
     Result eval(const vespalib::string &expr, const vespalib::string &name = {}, bool verbose = false) {
         Slime req;
         auto &obj = req.setObject();
@@ -70,6 +70,9 @@ struct Server : public ServerCmd {
         Slime reply = invoke(req);
         return {reply.get()};
     }
+    ~Server() {
+        EXPECT_EQUAL(shutdown(), 0);
+    }
 };
 
 //-----------------------------------------------------------------------------