Skip to content
This repository was archived by the owner on Aug 19, 2019. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ SED_EXTRA=-e 's/-Wall/-Wall -Wno-deprecated/'

UNAME_S=$(shell uname -s)
ifeq ($(UNAME_S),Darwin)
CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions \
-I/usr/local/include
CPPFLAGS+= -I/usr/local/include
CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
LDFLAGS+= -L/usr/local/lib
SED_I+= ''
SED_EXTRA+= -e \
Expand Down
2 changes: 2 additions & 0 deletions src/environment.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class Environment {
}

private:
friend class EnvironmentTest;

void ReadApplicationDefaultCredentials() const;

const Configuration& config_;
Expand Down
2 changes: 1 addition & 1 deletion src/json.cc
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ Parser::~Parser() = default;

std::size_t Parser::ParseStream(std::istream& stream) throw(Exception) {
const int kMax = 65536;
unsigned char data[kMax];
unsigned char data[kMax] = {0};
size_t total_bytes_consumed = 0;
yajl_handle handle = state_->handle();

Expand Down
14 changes: 12 additions & 2 deletions test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ LDLIBS=\
-lboost_program_options -lboost_system -lboost_thread -lboost_filesystem \
-lpthread -lyajl -lssl -lcrypto -lyaml-cpp

UNAME_S=$(shell uname -s)
ifeq ($(UNAME_S),Darwin)
CPPFLAGS+= -I/usr/local/include
CXXFLAGS+= -Wno-deprecated-declarations -Wno-c++14-extensions
LDFLAGS+= -L/usr/local/lib
endif

# Where to find code under test.
SRC_DIR=../src

Expand All @@ -42,6 +49,7 @@ TEST_OBJS=$(TEST_SOURCES:$(TEST_DIR)/%.cc=%.o)
TESTS=\
base64_unittest \
configuration_unittest \
environment_unittest \
format_unittest \
health_checker_unittest \
instance_unittest \
Expand Down Expand Up @@ -100,12 +108,14 @@ $(TESTS): $(GTEST_LIB) $(CPP_NETLIB_LIBS) $(YAML_CPP_LIBS)
# All unittest objects depend on GTEST_LIB.
$(TESTS:%=%.o): $(GTEST_LIB)

format_unittest: format_unittest.o $(SRC_DIR)/format.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
base64_unittest: base64_unittest.o $(SRC_DIR)/base64.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
configuration_unittest: configuration_unittest.o $(SRC_DIR)/configuration.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
environment_unittest: environment_unittest.o $(SRC_DIR)/environment.o $(SRC_DIR)/configuration.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/time.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
format_unittest: format_unittest.o $(SRC_DIR)/format.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -o $@
health_checker_unittest: health_checker_unittest.o $(SRC_DIR)/health_checker.o $(SRC_DIR)/configuration.o
$(CXX) $(LDFLAGS) $^ $(LDLIBS) -lboost_filesystem -lboost_system -o $@
instance_unittest: instance_unittest.o $(SRC_DIR)/instance.o $(SRC_DIR)/configuration.o $(SRC_DIR)/environment.o $(SRC_DIR)/format.o $(SRC_DIR)/json.o $(SRC_DIR)/logging.o $(SRC_DIR)/resource.o $(SRC_DIR)/store.o $(SRC_DIR)/time.o
Expand Down
101 changes: 101 additions & 0 deletions test/environment_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
#include "../src/environment.h"
#include "gtest/gtest.h"

#include <fstream>
#include <sstream>
#include <boost/filesystem.hpp>

namespace google {

class EnvironmentTest : public ::testing::Test {
protected:
static void ReadApplicationDefaultCredentials(const Environment& environment) {
environment.ReadApplicationDefaultCredentials();
}
};

namespace {
// A file with a given name in a temporary (unique) directory.
boost::filesystem::path TempPath(const std::string& filename) {
boost::filesystem::path path = boost::filesystem::temp_directory_path();
path.append(boost::filesystem::unique_path().native());
path.append(filename);
return path;
}

// Creates a file for the lifetime of the object and removes it after.
class TemporaryFile {
public:
TemporaryFile(const std::string& filename, const std::string& contents)
: path_(TempPath(filename)) {
boost::filesystem::create_directories(path_.parent_path());
SetContents(contents);
}
~TemporaryFile() {
boost::filesystem::remove_all(path_.parent_path());
}
void SetContents(const std::string& contents) const {
std::ofstream file(path_.native());
file << contents << std::flush;
}
const boost::filesystem::path& FullPath() const { return path_; }
private:
boost::filesystem::path path_;
};
} // namespace

TEST(TemporaryFile, Basic) {
boost::filesystem::path path;
{
TemporaryFile f("foo", "bar");
path = f.FullPath();
EXPECT_TRUE(boost::filesystem::exists(path));
std::string contents;
{
std::ifstream in(path.native());
in >> contents;
}
EXPECT_EQ("bar", contents);
f.SetContents("xyz");
{
std::ifstream in(path.native());
in >> contents;
}
EXPECT_EQ("xyz", contents);
}
EXPECT_FALSE(boost::filesystem::exists(path));
}

TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsSucceeds) {
TemporaryFile credentials_file(
std::string(test_info_->name()) + "_creds.json",
Copy link
Contributor

@supriyagarg supriyagarg Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where does test_info_ come from?

Copy link
Contributor

@ACEmilG ACEmilG Apr 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_info_ is provided by gtest.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Umm, yeah, except the preferred way to access it seems to be by calling current_test_info() on UnitTest. I'll send a separate PR with a fix.

"{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}");
std::string cfg;
Configuration config(std::istringstream(
"CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
));
Environment environment(config);
EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion this line is not required. It will be called on any call to CredentialsClientEmail or CredentialsPrivateKey. If it throws, the test case will exit and fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, but I'd like to control where the credentials file is actually read. Especially in the next test.

EXPECT_EQ("user@example.com", environment.CredentialsClientEmail());
EXPECT_EQ("some_key", environment.CredentialsPrivateKey());
}

TEST_F(EnvironmentTest, ReadApplicationDefaultCredentialsCaches) {
TemporaryFile credentials_file(
std::string(test_info_->name()) + "_creds.json",
"{\"client_email\":\"user@example.com\",\"private_key\":\"some_key\"}");
Configuration config(std::istringstream(
"CredentialsFile: '" + credentials_file.FullPath().native() + "'\n"
));
Environment environment(config);
EXPECT_NO_THROW(ReadApplicationDefaultCredentials(environment));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would personally change this line to something like EXPECT_EQ("foo@bar.com", environment.CredentialsClientEmail());. This will 1) load the cache and 2) demonstrate the expectation (that the cache is loaded) to the test-reader.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above. This is checking that the first call to ReadApplicationDefaultCredentials caches both the client email and the private key values.

credentials_file.SetContents(
"{\"client_email\":\"changed@example.com\",\"private_key\":\"12345\"}"
);
EXPECT_EQ("user@example.com", environment.CredentialsClientEmail());
credentials_file.SetContents(
"{\"client_email\":\"extra@example.com\",\"private_key\":\"09876\"}"
);
EXPECT_EQ("some_key", environment.CredentialsPrivateKey());
}
} // namespace google
1 change: 0 additions & 1 deletion test/health_checker_unittest.cc
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
#include "../src/health_checker.h"
#include "gtest/gtest.h"
#include <stdio.h>
#include <sstream>
#include <boost/filesystem.hpp>

Expand Down