Jsonnet: More test improvements

- Integrate `JsonnetTestHelper` into `JsonnetTest` class, remov multiple
  inheritance
- Do not copy strings where it can be avoided
- Add missing includes to header files

contrib/jsonnet/CMakeLists.txt

@@ -65,20 +65,18 @@ if(SAPI_ENABLE_TESTS)
   enable_testing()
 
   # Create directories so the tests will be able to access them
-  file(MAKE_DIRECTORY "${PROJECT_BINARY_DIR}/tests/tests_input"
+  file(MAKE_DIRECTORY "${PROJECT_BINARY_DIR}/tests_input"
-                      "${PROJECT_BINARY_DIR}/tests/tests_output"
+                      "${PROJECT_BINARY_DIR}/tests_output"
-                      "${PROJECT_BINARY_DIR}/tests/tests_expected_output")
+                      "${PROJECT_BINARY_DIR}/tests_expected_output")
 
   add_custom_target(test_preparation ALL
     COMMAND cp ${PROJECT_SOURCE_DIR}/examples/jsonnet_codes/*
-              ${PROJECT_BINARY_DIR}/tests/tests_input
+              ${PROJECT_BINARY_DIR}/tests_input
     COMMAND cp ${PROJECT_SOURCE_DIR}/examples/jsonnet_codes_expected_output/*
-              ${PROJECT_BINARY_DIR}/tests/tests_expected_output
+              ${PROJECT_BINARY_DIR}/tests_expected_output
   )
 
   add_executable(jsonnet_tests
-    jsonnet_test_helper.cc
-    jsonnet_test_helper.h
     jsonnet_tests.cc
   )
   target_include_directories(jsonnet_tests PUBLIC

contrib/jsonnet/README.md

@@ -69,9 +69,9 @@ directory, in a file called `formatter_example.jsonnet`.
 ## Testing
 
 A few tests prepared with a use of
-[Google Test](https://github.com/google/googletest) framework can be found in
+[Google Test](https://github.com/google/googletest) framework are included. To
-the `tests/` directory. To run them type:
+run them type:
 
 ```
-ctest ./tests
+ctest -R JsonnetTest.
 ```

contrib/jsonnet/jsonnet_base_sandbox.h

@@ -18,15 +18,19 @@
 #include <libgen.h>
 #include <syscall.h>
 
+#include <memory>
+#include <string>
+#include <utility>
+
 #include "jsonnet_sapi.sapi.h"  // NOLINT(build/include)
-#include "sandboxed_api/util/flag.h"
 #include "sandboxed_api/transaction.h"
+#include "sandboxed_api/util/flag.h"
 #include "sandboxed_api/vars.h"
 
 class JsonnetBaseSandbox : public JsonnetSandbox {
  public:
   explicit JsonnetBaseSandbox(std::string in_file, std::string out_file)
-      : in_file_(in_file), out_file_(out_file) {}
+      : in_file_(std::move(in_file)), out_file_(std::move(out_file)) {}
 
   std::unique_ptr<sandbox2::Policy> ModifyPolicy(
       sandbox2::PolicyBuilder *) override {

contrib/jsonnet/jsonnet_base_transaction.h

@@ -15,6 +15,9 @@
 #ifndef CONTRIB_JSNONNET_BASE_TRANSACTION_H_
 #define CONTRIB_JSNONNET_BASE_TRANSACTION_H_
 
+#include <memory>
+#include <string>
+
 #include "jsonnet_base_sandbox.h"  // NOLINT(build/include)
 
 class JsonnetTransaction : public sapi::Transaction {

contrib/jsonnet/jsonnet_test_helper.cc

@@ -1,163 +0,0 @@
-// Copyright 2020 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "jsonnet_test_helper.h"  // NOLINT(build/include)
-
-// Prepares what is needed to perform a test.
-void JsonnetTestHelper::TestSetUp() {
-  // Get paths to where input and output is stored.
-  char buffer[256];
-  int error = readlink("/proc/self/exe", buffer, 256);
-  ASSERT_GE(error, 0);
-
-  std::pair<absl::string_view, absl::string_view> parts_of_path =
-      sapi::file::SplitPath(buffer);
-  absl::string_view binary_path = parts_of_path.first;
-
-  std::string input_path =
-      sapi::file::JoinPath(binary_path, "tests_input", "dummy_input");
-  std::string output_path =
-      sapi::file::JoinPath(binary_path, "tests_output", "dummy_input");
-
-  // Set up sandbox and api.
-  sandbox_ = absl::make_unique<JsonnetBaseSandbox>(input_path, output_path);
-  ASSERT_THAT(sandbox_->Init(), sapi::IsOk());
-  api_ = absl::make_unique<JsonnetApi>(sandbox_.get());
-
-  // Initialize library's main structure.
-  SAPI_ASSERT_OK_AND_ASSIGN(JsonnetVm * vm_ptr, api_->c_jsonnet_make());
-  vm_ = absl::make_unique<sapi::v::RemotePtr>(vm_ptr);
-
-  jsonnet_vm_was_used_ = false;
-  input_was_read_ = false;
-}
-
-// Cleans up after a test.
-void JsonnetTestHelper::TestTearDown() {
-  if (jsonnet_vm_was_used_) {
-    SAPI_ASSERT_OK_AND_ASSIGN(
-        char* result, api_->c_jsonnet_realloc(vm_.get(), output_.get(), 0));
-  }
-  ASSERT_THAT(api_->c_jsonnet_destroy(vm_.get()), sapi::IsOk());
-  if (input_was_read_) {
-    ASSERT_THAT(api_->c_free_input(input_.get()), sapi::IsOk());
-  }
-}
-
-// Reads input from file.
-void JsonnetTestHelper::ReadInput(const char* filename) {
-  std::string in_file_in_sandboxee(std::string("/input/") +
-                                   basename(const_cast<char*>(&filename[0])));
-  input_filename_in_sandboxee_ = std::move(in_file_in_sandboxee);
-  sapi::v::ConstCStr in_file_var(input_filename_in_sandboxee_.c_str());
-
-  SAPI_ASSERT_OK_AND_ASSIGN(char* input_ptr,
-                            api_->c_read_input(0, in_file_var.PtrBefore()));
-  input_ = absl::make_unique<sapi::v::RemotePtr>(input_ptr);
-
-  input_was_read_ = true;
-}
-
-// Evaluates jsonnet code.
-void JsonnetTestHelper::EvaluateJsonnetCode(Evaluation type,
-                                            bool expected_correct) {
-  sapi::v::ConstCStr in_file_var(input_filename_in_sandboxee_.c_str());
-  sapi::v::Int error;
-  char* output_ptr;
-
-  switch (type) {
-    case kBase: {
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          output_ptr,
-          api_->c_jsonnet_evaluate_snippet(vm_.get(), in_file_var.PtrBefore(),
-                                           input_.get(), error.PtrAfter()));
-      break;
-    }
-
-    case kMultipleFiles: {
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          output_ptr, api_->c_jsonnet_evaluate_snippet_multi(
-                          vm_.get(), in_file_var.PtrBefore(), input_.get(),
-                          error.PtrAfter()));
-      break;
-    }
-
-    case kYamlStream: {
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          output_ptr, api_->c_jsonnet_evaluate_snippet_stream(
-                          vm_.get(), in_file_var.PtrBefore(), input_.get(),
-                          error.PtrAfter()));
-      break;
-    }
-  }
-
-  if (expected_correct) {
-    ASSERT_THAT(error.GetValue(), testing::Eq(0));
-  } else {
-    ASSERT_THAT(error.GetValue(), testing::Eq(1));
-  }
-
-  output_ = absl::make_unique<sapi::v::RemotePtr>(output_ptr);
-
-  jsonnet_vm_was_used_ = true;
-}
-
-// Writes output to file.
-void JsonnetTestHelper::WriteOutput(const char* filename_or_directory,
-                                    Evaluation type) {
-  bool success;
-
-  switch (type) {
-    case kBase: {
-      std::string out_file_in_sandboxee(
-          std::string("/output/") +
-          basename(const_cast<char*>(&filename_or_directory[0])));
-      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
-
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          success,
-          api_->c_write_output_file(output_.get(), out_file_var.PtrBefore()));
-      break;
-    }
-    case kMultipleFiles: {
-      std::string out_file_in_sandboxee(std::string("/output/"));
-      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          success, api_->c_write_multi_output_files(
-                       output_.get(), out_file_var.PtrBefore(), false));
-      break;
-    }
-
-    case kYamlStream: {
-      std::string out_file_in_sandboxee(
-          std::string("/output/") +
-          basename(const_cast<char*>(&filename_or_directory[0])));
-      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
-      SAPI_ASSERT_OK_AND_ASSIGN(
-          success,
-          api_->c_write_output_stream(output_.get(), out_file_var.PtrBefore()));
-      break;
-    }
-  }
-
-  ASSERT_THAT(success, testing::Eq(true));
-}
-
-// Reads the output written to a file by library function / expected output
-std::string JsonnetTestHelper::ReadOutput(const char* filename) {
-  std::ifstream input_stream(filename);
-  std::string contents((std::istreambuf_iterator<char>(input_stream)),
-                       std::istreambuf_iterator<char>());
-  return contents;
-}

contrib/jsonnet/jsonnet_test_helper.h

@@ -1,57 +0,0 @@
-// Copyright 2020 Google LLC
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef CONTRIB_JSONNET_TESTS_H_
-#define CONTRIB_JSONNET_TESTS_H_
-
-#include <unistd.h>
-
-#include <filesystem>
-#include <fstream>
-#include <iostream>
-#include <memory>
-#include <streambuf>
-#include <string>
-
-#include "jsonnet_base_sandbox.h"  // NOLINT(build/include)
-#include "jsonnet_sapi.sapi.h"     // NOLINT(build/include)
-#include "gtest/gtest.h"
-#include "sandboxed_api/util/flag.h"
-#include "sandboxed_api/util/path.h"
-#include "sandboxed_api/util/status_matchers.h"
-
-class JsonnetTestHelper {
- protected:
-  enum Evaluation { kBase, kMultipleFiles, kYamlStream };
-
-  void TestSetUp();
-  void TestTearDown();
-
-  void ReadInput(const char* filename);
-  void EvaluateJsonnetCode(Evaluation type, bool expected_correct);
-  void WriteOutput(const char* filename_or_directory, Evaluation type);
-  std::string ReadOutput(const char* filename);
-
-  std::unique_ptr<JsonnetBaseSandbox> sandbox_;
-  std::unique_ptr<JsonnetApi> api_;
-  std::unique_ptr<sapi::v::RemotePtr> input_;
-  std::unique_ptr<sapi::v::RemotePtr> output_;
-  std::unique_ptr<sapi::v::RemotePtr> vm_;
-
-  std::string input_filename_in_sandboxee_;
-  bool jsonnet_vm_was_used_;
-  bool input_was_read_;
-};
-
-#endif  // CONTRIB_JSONNET_TESTS_H_

contrib/jsonnet/jsonnet_tests.cc

@@ -12,20 +12,186 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-#include "jsonnet_test_helper.h"  // NOLINT(build/include)
+#include <unistd.h>
+
+#include <fstream>
+#include <iostream>
+#include <memory>
+#include <streambuf>
+#include <string>
+
+#include "gtest/gtest.h"
+#include "jsonnet_base_sandbox.h"  // NOLINT(build/include)
+#include "jsonnet_sapi.sapi.h"     // NOLINT(build/include)
+#include "sandboxed_api/util/flag.h"
+#include "sandboxed_api/util/path.h"
+#include "sandboxed_api/util/status_matchers.h"
 
 namespace {
 
-class JsonnetTest : public JsonnetTestHelper, public testing::Test {
+class JsonnetTest : public ::testing::Test {
  protected:
-  void SetUp() override { TestSetUp(); }
+  enum Evaluation { kBase, kMultipleFiles, kYamlStream };
-  void TearDown() override { TestTearDown(); }
+
+  void SetUp() override {
+    // Get paths to where input and output is stored.
+    char buffer[256];
+    int error = readlink("/proc/self/exe", buffer, 256);
+    ASSERT_GE(error, 0);
+
+    std::pair<absl::string_view, absl::string_view> parts_of_path =
+        sapi::file::SplitPath(buffer);
+    absl::string_view binary_path = parts_of_path.first;
+
+    std::string input_path =
+        sapi::file::JoinPath(binary_path, "tests_input", "dummy_input");
+    std::string output_path =
+        sapi::file::JoinPath(binary_path, "tests_output", "dummy_input");
+
+    // Set up sandbox and api.
+    sandbox_ = absl::make_unique<JsonnetBaseSandbox>(input_path, output_path);
+    ASSERT_THAT(sandbox_->Init(), sapi::IsOk());
+    api_ = absl::make_unique<JsonnetApi>(sandbox_.get());
+
+    // Initialize library's main structure.
+    SAPI_ASSERT_OK_AND_ASSIGN(JsonnetVm* vm_ptr, api_->c_jsonnet_make());
+    vm_ = absl::make_unique<sapi::v::RemotePtr>(vm_ptr);
+  }
+
+  void TearDown() override {
+    if (jsonnet_vm_was_used_) {
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          char* result, api_->c_jsonnet_realloc(vm_.get(), output_.get(), 0));
+    }
+    ASSERT_THAT(api_->c_jsonnet_destroy(vm_.get()), sapi::IsOk());
+    if (input_was_read_) {
+      ASSERT_THAT(api_->c_free_input(input_.get()), sapi::IsOk());
+    }
+  }
+
+  // Reads input from file.
+  void ReadInput(const char* filename);
+
+  // Evaluates jsonnet code.
+  void EvaluateJsonnetCode(Evaluation type, bool expected_correct);
+
+  // Writes output to file.
+  void WriteOutput(const char* filename_or_directory, Evaluation type);
+
+  // Reads the output written to a file by library function / expected output.
+  std::string ReadOutput(const char* filename);
+
+  std::unique_ptr<JsonnetBaseSandbox> sandbox_;
+  std::unique_ptr<JsonnetApi> api_;
+  std::unique_ptr<sapi::v::RemotePtr> input_;
+  std::unique_ptr<sapi::v::RemotePtr> output_;
+  std::unique_ptr<sapi::v::RemotePtr> vm_;
+
+  std::string input_filename_in_sandboxee_;
+  bool jsonnet_vm_was_used_ = false;
+  bool input_was_read_ = false;
 };
 
-// Basic test
+void JsonnetTest::ReadInput(const char* filename) {
-TEST_F(JsonnetTest, SetUp_TearDown) {
+  std::string in_file_in_sandboxee(std::string("/input/") +
-  ASSERT_FALSE(jsonnet_vm_was_used_);
+                                   basename(const_cast<char*>(&filename[0])));
-  ASSERT_FALSE(input_was_read_);
+  input_filename_in_sandboxee_ = std::move(in_file_in_sandboxee);
+  sapi::v::ConstCStr in_file_var(input_filename_in_sandboxee_.c_str());
+
+  SAPI_ASSERT_OK_AND_ASSIGN(char* input_ptr,
+                            api_->c_read_input(0, in_file_var.PtrBefore()));
+  input_ = absl::make_unique<sapi::v::RemotePtr>(input_ptr);
+
+  input_was_read_ = true;
+}
+
+void JsonnetTest::EvaluateJsonnetCode(Evaluation type, bool expected_correct) {
+  sapi::v::ConstCStr in_file_var(input_filename_in_sandboxee_.c_str());
+  sapi::v::Int error;
+  char* output_ptr;
+
+  switch (type) {
+    case kBase: {
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          output_ptr,
+          api_->c_jsonnet_evaluate_snippet(vm_.get(), in_file_var.PtrBefore(),
+                                           input_.get(), error.PtrAfter()));
+      break;
+    }
+
+    case kMultipleFiles: {
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          output_ptr, api_->c_jsonnet_evaluate_snippet_multi(
+                          vm_.get(), in_file_var.PtrBefore(), input_.get(),
+                          error.PtrAfter()));
+      break;
+    }
+
+    case kYamlStream: {
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          output_ptr, api_->c_jsonnet_evaluate_snippet_stream(
+                          vm_.get(), in_file_var.PtrBefore(), input_.get(),
+                          error.PtrAfter()));
+      break;
+    }
+  }
+
+  if (expected_correct) {
+    ASSERT_THAT(error.GetValue(), testing::Eq(0));
+  } else {
+    ASSERT_THAT(error.GetValue(), testing::Eq(1));
+  }
+
+  output_ = absl::make_unique<sapi::v::RemotePtr>(output_ptr);
+
+  jsonnet_vm_was_used_ = true;
+}
+
+void JsonnetTest::WriteOutput(const char* filename_or_directory,
+                              Evaluation type) {
+  bool success;
+
+  switch (type) {
+    case kBase: {
+      std::string out_file_in_sandboxee(
+          std::string("/output/") +
+          basename(const_cast<char*>(&filename_or_directory[0])));
+      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
+
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          success,
+          api_->c_write_output_file(output_.get(), out_file_var.PtrBefore()));
+      break;
+    }
+    case kMultipleFiles: {
+      std::string out_file_in_sandboxee(std::string("/output/"));
+      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          success, api_->c_write_multi_output_files(
+                       output_.get(), out_file_var.PtrBefore(), false));
+      break;
+    }
+
+    case kYamlStream: {
+      std::string out_file_in_sandboxee(
+          std::string("/output/") +
+          basename(const_cast<char*>(&filename_or_directory[0])));
+      sapi::v::ConstCStr out_file_var(out_file_in_sandboxee.c_str());
+      SAPI_ASSERT_OK_AND_ASSIGN(
+          success,
+          api_->c_write_output_stream(output_.get(), out_file_var.PtrBefore()));
+      break;
+    }
+  }
+
+  ASSERT_THAT(success, testing::Eq(true));
+}
+
+std::string JsonnetTest::ReadOutput(const char* filename) {
+  std::ifstream input_stream(filename);
+  std::string contents((std::istreambuf_iterator<char>(input_stream)),
+                       std::istreambuf_iterator<char>());
+  return contents;
 }
 
 // One file evaluation to one file