From 830520936f77ba1b5b671822271e5c96a675bd9a Mon Sep 17 00:00:00 2001 From: Ivan Petrov Date: Mon, 13 Jan 2020 14:53:34 +0000 Subject: [PATCH] Add ApplicationConfig tests --- oak/common/app_config.cc | 8 ++-- oak/common/app_config.h | 8 ++-- oak/common/app_config_serializer.cc | 31 +++++++------- oak/common/app_config_test.cc | 65 +++++++++++++++++++---------- oak/common/utils.cc | 17 ++++---- oak/common/utils.h | 6 +-- 6 files changed, 74 insertions(+), 61 deletions(-) diff --git a/oak/common/app_config.cc b/oak/common/app_config.cc index e112d2c13c6..70cc8aa0514 100644 --- a/oak/common/app_config.cc +++ b/oak/common/app_config.cc @@ -46,18 +46,18 @@ std::unique_ptr DefaultConfig(const std::string& modul return config; } -std::unique_ptr ReadConfigFromFile(const std::string& file) { +std::unique_ptr ReadConfigFromFile(const std::string& filename) { auto config = absl::make_unique(); - std::string data = utils::read_file(file); + std::string data = utils::read_file(filename); config->ParseFromString(data); return config; } -void WriteConfigToFile(const ApplicationConfiguration* config, const std::string& file) { +void WriteConfigToFile(const ApplicationConfiguration* config, const std::string& filename) { std::string data = config->SerializeAsString(); - utils::write_file(data, file); + utils::write_file(data, filename); } void AddLoggingToConfig(ApplicationConfiguration* config) { diff --git a/oak/common/app_config.h b/oak/common/app_config.h index 7b7b144c868..f3cb42c07e9 100644 --- a/oak/common/app_config.h +++ b/oak/common/app_config.h @@ -27,11 +27,11 @@ namespace oak { // name and contents, accessible via gRPC. std::unique_ptr DefaultConfig(const std::string& module_bytes); -// Reads a serialized application configuration from `file`. -std::unique_ptr ReadConfigFromFile(const std::string& file); +// Reads a serialized application configuration from file. +std::unique_ptr ReadConfigFromFile(const std::string& filename); -// Serializes an application configuration from `config` and writes it into a `file`. -void WriteConfigToFile(const ApplicationConfiguration* config, const std::string& file); +// Serializes an application configuration from `config` and writes it into a file. +void WriteConfigToFile(const ApplicationConfiguration* config, const std::string& filename); // Modify application configuration to make logging available. void AddLoggingToConfig(ApplicationConfiguration* config); diff --git a/oak/common/app_config_serializer.cc b/oak/common/app_config_serializer.cc index dd883fd5a1c..ca0b04b3980 100644 --- a/oak/common/app_config_serializer.cc +++ b/oak/common/app_config_serializer.cc @@ -14,16 +14,14 @@ * limitations under the License. */ -#include "oak/common/app_config.h" -#include "oak/common/utils.h" - -#include #include #include #include "absl/flags/flag.h" #include "absl/flags/parse.h" #include "asylo/util/logging.h" +#include "oak/common/app_config.h" +#include "oak/common/utils.h" ABSL_FLAG(std::string, module, "", "File containing the compiled WebAssembly module"); ABSL_FLAG(bool, logging, false, "Enable application logging"); @@ -32,21 +30,19 @@ ABSL_FLAG(std::string, storage_address, "127.0.0.1:7867", ABSL_FLAG(int, grpc_port, 8080, "Port for the Application to listen on"); ABSL_FLAG(std::string, output_file, "", "File to write an application configuration to"); -void sigint_handler(int) { - LOG(QFATAL) << "SIGINT received"; - exit(1); -} - int main(int argc, char* argv[]) { absl::ParseCommandLine(argc, argv); - // We install an explicit SIGINT handler, as for some reason the default one - // does not seem to work. - std::signal(SIGINT, sigint_handler); + std::string output_file = absl::GetFlag(FLAGS_output_file); + if (output_file.empty()) { + LOG(QFATAL) << "Output file is not specified"; + return 1; + } // Create an application configuration. std::string module_bytes = oak::utils::read_file(absl::GetFlag(FLAGS_module)); - auto application_config = oak::DefaultConfig(module_bytes); + std::unique_ptr application_config = oak::DefaultConfig( + module_bytes); // Set application configuration parameters. if (absl::GetFlag(FLAGS_logging)) { @@ -55,12 +51,13 @@ int main(int argc, char* argv[]) { oak::AddStorageToConfig(application_config.get(), absl::GetFlag(FLAGS_storage_address)); oak::AddGrpcPortToConfig(application_config.get(), absl::GetFlag(FLAGS_grpc_port)); - std::string output_file = absl::GetFlag(FLAGS_output_file); - if (output_file.empty()) { - LOG(QFATAL) << "Output file is not specified"; + if (ValidApplicationConfig(application_config.get())) { + oak::WriteConfigToFile(application_config.get(), output_file); + } + else { + LOG(QFATAL) << "Incorrect application configuration"; return 1; } - oak::WriteConfigToFile(application_config.get(), output_file); return 0; } diff --git a/oak/common/app_config_test.cc b/oak/common/app_config_test.cc index 298a38eba91..838608aa6ce 100644 --- a/oak/common/app_config_test.cc +++ b/oak/common/app_config_test.cc @@ -27,21 +27,30 @@ namespace oak { namespace { -std::unique_ptr ConfigFrom(const std::string& filename) { - std::ifstream ifs(filename.c_str(), std::ios::in | std::ios::binary); - EXPECT_TRUE(ifs.is_open()) << "failed to open " << filename; - std::stringstream ss; - ss << ifs.rdbuf(); - ifs.close(); - - auto config = absl::make_unique(); - google::protobuf::TextFormat::MergeFromString(ss.str(), config.get()); - return config; -} - -} // namespace - -TEST(ApplicationConfiguration, Default) { +// Fixture class for testing `ApplicationConfiguration` correctness. +class ApplicationConfigurationTest : public ::testing::Test { + protected: + virtual void TearDown() { + // Clean up temporary files. + std::remove(kTmpFilename.c_str()); + } + + std::unique_ptr ConfigFrom(const std::string& filename) { + std::ifstream ifs(filename.c_str(), std::ios::in | std::ios::binary); + EXPECT_TRUE(ifs.is_open()) << "failed to open " << filename; + std::stringstream ss; + ss << ifs.rdbuf(); + ifs.close(); + + auto config = absl::make_unique(); + google::protobuf::TextFormat::MergeFromString(ss.str(), config.get()); + return config; + } + + const std::string kTmpFilename = "oak/common/testdata/tmp.bin"; +}; + +TEST_F(ApplicationConfigurationTest, Default) { std::unique_ptr got = DefaultConfig(""); std::unique_ptr want = ConfigFrom("oak/common/testdata/barenode.textproto"); @@ -49,7 +58,15 @@ TEST(ApplicationConfiguration, Default) { ASSERT_EQ(true, ValidApplicationConfig(*got)); } -TEST(ApplicationConfiguration, DefaultPlusLogging) { +TEST_F(ApplicationConfigurationTest, ReadWriteFile) { + std::unique_ptr want = DefaultConfig(""); + WriteConfigToFile(want.get(), kTmpFilename); + std::unique_ptr got = ReadConfigFromFile(kTmpFilename); + ASSERT_EQ(want->DebugString(), got->DebugString()); + ASSERT_EQ(true, ValidApplicationConfig(*got)); +} + +TEST_F(ApplicationConfigurationTest, DefaultPlusLogging) { std::unique_ptr got = DefaultConfig(""); AddLoggingToConfig(got.get()); std::unique_ptr want = @@ -58,7 +75,7 @@ TEST(ApplicationConfiguration, DefaultPlusLogging) { ASSERT_EQ(true, ValidApplicationConfig(*got)); } -TEST(ApplicationConfiguration, DefaultPlusStorage) { +TEST_F(ApplicationConfigurationTest, DefaultPlusStorage) { std::unique_ptr got = DefaultConfig(""); AddStorageToConfig(got.get(), "localhost:8888"); std::unique_ptr want = @@ -67,7 +84,7 @@ TEST(ApplicationConfiguration, DefaultPlusStorage) { ASSERT_EQ(true, ValidApplicationConfig(*got)); } -TEST(ApplicationConfiguration, DefaultPlusGrpcPort) { +TEST_F(ApplicationConfigurationTest, DefaultPlusGrpcPort) { std::unique_ptr got = DefaultConfig(""); AddGrpcPortToConfig(got.get(), 8080); std::unique_ptr want = @@ -76,30 +93,32 @@ TEST(ApplicationConfiguration, DefaultPlusGrpcPort) { ASSERT_EQ(true, ValidApplicationConfig(*got)); } -TEST(ApplicationConfiguration, Valid) { +TEST_F(ApplicationConfigurationTest, Valid) { auto config = ConfigFrom("oak/common/testdata/default.textproto"); ASSERT_EQ(true, ValidApplicationConfig(*config)); } -TEST(ApplicationConfiguration, DuplicateConfigName) { +TEST_F(ApplicationConfigurationTest, DuplicateConfigName) { auto config = ConfigFrom("oak/common/testdata/dup_config_name.textproto"); ASSERT_EQ(false, ValidApplicationConfig(*config)); } -TEST(ApplicationConfiguration, MultipleLogNodes) { +TEST_F(ApplicationConfigurationTest, MultipleLogNodes) { // Two log configs are OK. auto config = ConfigFrom("oak/common/testdata/two_log.textproto"); ASSERT_EQ(true, ValidApplicationConfig(*config)); } -TEST(ApplicationConfiguration, NonWasmCode) { +TEST_F(ApplicationConfigurationTest, NonWasmCode) { auto config = ConfigFrom("oak/common/testdata/missing_wasm.textproto"); ASSERT_EQ(false, ValidApplicationConfig(*config)); } -TEST(ApplicationConfiguration, DuplicateWasmName) { +TEST_F(ApplicationConfigurationTest, DuplicateWasmName) { auto config = ConfigFrom("oak/common/testdata/dup_wasm.textproto"); ASSERT_EQ(false, ValidApplicationConfig(*config)); } +} // namespace + } // namespace oak diff --git a/oak/common/utils.cc b/oak/common/utils.cc index 054d932562c..3b82b417421 100644 --- a/oak/common/utils.cc +++ b/oak/common/utils.cc @@ -14,31 +14,28 @@ * limitations under the License. */ -#include "oak/common/utils.h" - #include #include "asylo/util/logging.h" +#include "oak/common/utils.h" namespace oak { namespace utils { -// Reads a binary file and returns its contents as a std::string. -std::string read_file(const std::string& module_path) { - std::ifstream t(module_path, std::ifstream::in); +std::string read_file(const std::string& filename) { + std::ifstream t(filename, std::ifstream::in); if (!t.is_open()) { - LOG(QFATAL) << "Could not open module " << module_path; + LOG(QFATAL) << "Could not open file " << filename; } std::stringstream buffer; buffer << t.rdbuf(); return buffer.str(); } -// Writes `data` string into a binary `file`. -void write_file(const std::string& data, const std::string& file) { - std::ofstream t(file, std::ofstream::out); +void write_file(const std::string& data, const std::string& filename) { + std::ofstream t(filename, std::ofstream::out); if (!t.is_open()) { - LOG(QFATAL) << "Could not open file " << file; + LOG(QFATAL) << "Could not open file " << filename; } t << data; t.close(); diff --git a/oak/common/utils.h b/oak/common/utils.h index a9932631f11..7aaf1fc61ea 100644 --- a/oak/common/utils.h +++ b/oak/common/utils.h @@ -23,10 +23,10 @@ namespace oak { namespace utils { // Reads a binary file and returns its contents as a std::string. -std::string read_file(const std::string& module_path); +std::string read_file(const std::string& filename); -// Writes `data` string into a binary `file`. -void write_file(const std::string& data, const std::string& file); +// Writes `data` string into a binary file. +void write_file(const std::string& data, const std::string& filename); } // namespace utils } // namespace oak