From 58ed1e6d6842688ac2d32355265e0d89049f7e36 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Tue, 5 May 2020 17:56:46 +0200 Subject: [PATCH 01/12] WIP: add unit tests for libutil This is a proof on concept to evaluate writing unit tests for Nix using google test (https://github.com/google/googletest). In order to execute tests: $ make unit-tests $ ./unit-tests The Makefile rules for `unit-tests` is a complete hack. --- Makefile | 3 +- release-common.nix | 1 + tests/unit-tests/local.mk | 4 + tests/unit-tests/test-util.cc | 594 ++++++++++++++++++++++++++++++++++ 4 files changed, 601 insertions(+), 1 deletion(-) create mode 100644 tests/unit-tests/local.mk create mode 100644 tests/unit-tests/test-util.cc diff --git a/Makefile b/Makefile index e3057c36ca2..06de35a2709 100644 --- a/Makefile +++ b/Makefile @@ -16,7 +16,8 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk + tests/plugins/local.mk \ + tests/unit-tests/local.mk -include Makefile.config diff --git a/release-common.nix b/release-common.nix index b6c8f3d818d..7e7de005d2f 100644 --- a/release-common.nix +++ b/release-common.nix @@ -55,6 +55,7 @@ rec { # Tests git mercurial + gmock ] ++ lib.optionals stdenv.isLinux [libseccomp utillinuxMinimal] ++ lib.optional (stdenv.isLinux || stdenv.isDarwin) libsodium diff --git a/tests/unit-tests/local.mk b/tests/unit-tests/local.mk new file mode 100644 index 00000000000..ed11261c1a0 --- /dev/null +++ b/tests/unit-tests/local.mk @@ -0,0 +1,4 @@ +LIBS=-larchive -lcrypto -llzma -lbz2 -lz -lbrotlienc -lbrotlidec +unit-tests: + echo $(nix_LDFLAGS) + $(CXX) -o unit-test $(nix_CXXFLAGS) $(nix_LDFLAGS) $(LIBS) --std=c++17 $$(pkg-config --libs gtest_main) ./src/libutil/*.o tests/unit-tests/test-util.cc diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc new file mode 100644 index 00000000000..05d8a0039d9 --- /dev/null +++ b/tests/unit-tests/test-util.cc @@ -0,0 +1,594 @@ +#include "../../src/libutil/util.hh" +#include "../../src/libutil/types.hh" + +#include + +namespace nix { + + /* ---------------------------------------------------------------------------- + * absPath + * --------------------------------------------------------------------------*/ + + TEST(absPath, doesntChangeRoot) { + auto p = absPath("/"); + + ASSERT_STREQ(p.c_str(), "/"); + } + + TEST(absPath, turnsEmptyPathIntoCWD) { + char cwd[PATH_MAX+1]; + auto p = absPath(""); + + ASSERT_STREQ(p.c_str(), getcwd((char*)&cwd, PATH_MAX)); + } + + TEST(absPath, usesOptionalBasePathWhenGiven) { + char _cwd[PATH_MAX+1]; + char* cwd = getcwd((char*)&_cwd, PATH_MAX); + + auto p = absPath("", cwd); + + ASSERT_STREQ(p.c_str(), cwd); + } + + TEST(absPath, isIdempotent) { + char _cwd[PATH_MAX+1]; + char* cwd = getcwd((char*)&_cwd, PATH_MAX); + auto p1 = absPath(cwd); + auto p2 = absPath(p1); + + ASSERT_STREQ(p1.c_str(), p2.c_str()); + } + + + TEST(absPath, pathIsCanonicalised) { + auto path = "/some/path/with/trailing/dot/."; + auto p1 = absPath(path); + auto p2 = absPath(p1); + + ASSERT_STREQ(p1.c_str(), "/some/path/with/trailing/dot"); + ASSERT_STREQ(p1.c_str(), p2.c_str()); + } + + /* ---------------------------------------------------------------------------- + * canonPath + * --------------------------------------------------------------------------*/ + + TEST(canonPath, removesTrailingSlashes) { + auto path = "/this/is/a/path//"; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, removesDots) { + auto path = "/this/./is/a/path/./"; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, removesDots2) { + auto path = "/this/a/../is/a////path/foo/.."; + auto p = canonPath(path); + + ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + } + + TEST(canonPath, requiresAbsolutePath) { + ASSERT_ANY_THROW(canonPath(".")); + ASSERT_ANY_THROW(canonPath("..")); + ASSERT_ANY_THROW(canonPath("../")); + ASSERT_DEATH({ canonPath(""); }, "path != \"\""); + } + + /* ---------------------------------------------------------------------------- + * dirOf + * --------------------------------------------------------------------------*/ + + // XXX: according to the doc of `dirOf`, dirOf("/") and dirOf("/foo") + // should both return "" but it actually returns "/" in both cases + TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { + auto p = dirOf("/"); + + ASSERT_STREQ(p.c_str(), ""); + } + + TEST(dirOf, returnsFirstPathComponent) { + auto p1 = dirOf("/dir/"); + ASSERT_STREQ(p1.c_str(), "/dir"); + auto p2 = dirOf("/dir"); + ASSERT_STREQ(p2.c_str(), "/"); + auto p3 = dirOf("/dir/.."); + ASSERT_STREQ(p3.c_str(), "/dir"); + auto p4 = dirOf("/dir/../"); + ASSERT_STREQ(p4.c_str(), "/dir/.."); + } + + /* ---------------------------------------------------------------------------- + * baseNameOf + * --------------------------------------------------------------------------*/ + + TEST(baseNameOf, emptyPath) { + auto p1 = baseNameOf(""); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + TEST(baseNameOf, pathOnRoot) { + auto p1 = baseNameOf("/dir"); + ASSERT_STREQ(std::string(p1).c_str(), "dir"); + } + + TEST(baseNameOf, relativePath) { + auto p1 = baseNameOf("dir/foo"); + ASSERT_STREQ(std::string(p1).c_str(), "foo"); + } + + TEST(baseNameOf, pathWithTrailingSlashRoot) { + auto p1 = baseNameOf("/"); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return + // "" but it actually returns "dir" + TEST(baseNameOf, DISABLED_trailingSlash) { + auto p1 = baseNameOf("/dir/"); + ASSERT_STREQ(std::string(p1).c_str(), ""); + } + + /* ---------------------------------------------------------------------------- + * isInDir + * --------------------------------------------------------------------------*/ + + TEST(isInDir, trivialCase) { + auto p1 = isInDir("/foo/bar", "/foo"); + ASSERT_EQ(p1, true); + } + + TEST(isInDir, notInDir) { + auto p1 = isInDir("/zes/foo/bar", "/foo"); + ASSERT_EQ(p1, false); + } + + // XXX: hm, bug or feature? :) Looking at the implementation + // this might be problematic. + TEST(isInDir, emptyDir) { + auto p1 = isInDir("/zes/foo/bar", ""); + ASSERT_EQ(p1, true); + } + + /* ---------------------------------------------------------------------------- + * isDirOrInDir + * --------------------------------------------------------------------------*/ + + TEST(isDirOrInDir, trueForSameDirectory) { + ASSERT_EQ(isDirOrInDir("/nix", "/nix"), true); + ASSERT_EQ(isDirOrInDir("/", "/"), true); + } + + TEST(isDirOrInDir, trueForEmptyPaths) { + ASSERT_EQ(isDirOrInDir("", ""), true); + } + + TEST(isDirOrInDir, falseForDisjunctPaths) { + ASSERT_EQ(isDirOrInDir("/foo", "/bar"), false); + } + + TEST(isDirOrInDir, relativePaths) { + ASSERT_EQ(isDirOrInDir("/foo/..", "/foo"), true); + } + + // XXX: while it is possible to use "." or ".." in the + // first argument this doesn't seem to work in the second. + TEST(isDirOrInDir, DISABLED_shouldWork) { + ASSERT_EQ(isDirOrInDir("/foo/..", "/foo/."), true); + + } + + /* ---------------------------------------------------------------------------- + * pathExists + * --------------------------------------------------------------------------*/ + + TEST(pathExists, rootExists) { + ASSERT_TRUE(pathExists("/")); + } + + TEST(pathExists, cwdExists) { + ASSERT_TRUE(pathExists(".")); + } + + TEST(pathExists, bogusPathDoesNotExist) { + ASSERT_FALSE(pathExists("/home/schnitzel/darmstadt/pommes")); + } + + /* ---------------------------------------------------------------------------- + * concatStringsSep + * --------------------------------------------------------------------------*/ + + TEST(concatStringsSep, buildCommaSeparatedString) { + Strings strings; + strings.push_back("this"); + strings.push_back("is"); + strings.push_back("great"); + + ASSERT_EQ(concatStringsSep(",", strings), "this,is,great"); + } + + TEST(concatStringsSep, buildStringWithEmptySeparator) { + Strings strings; + strings.push_back("this"); + strings.push_back("is"); + strings.push_back("great"); + + ASSERT_EQ(concatStringsSep("", strings), "thisisgreat"); + } + + TEST(concatStringsSep, buildSingleString) { + Strings strings; + strings.push_back("this"); + + ASSERT_EQ(concatStringsSep(",", strings), "this"); + } + + /* ---------------------------------------------------------------------------- + * hasPrefix + * --------------------------------------------------------------------------*/ + + TEST(hasPrefix, emptyStringHasNoPrefix) { + ASSERT_FALSE(hasPrefix("", "foo")); + } + + TEST(hasPrefix, emptyStringIsAlwaysPrefix) { + ASSERT_TRUE(hasPrefix("foo", "")); + ASSERT_TRUE(hasPrefix("jshjkfhsadf", "")); + } + + TEST(hasPrefix, trivialCase) { + ASSERT_TRUE(hasPrefix("foobar", "foo")); + } + + /* ---------------------------------------------------------------------------- + * hasSuffix + * --------------------------------------------------------------------------*/ + + TEST(hasSuffix, emptyStringHasNoSuffix) { + ASSERT_FALSE(hasSuffix("", "foo")); + } + + TEST(hasSuffix, trivialCase) { + ASSERT_TRUE(hasSuffix("foo", "foo")); + ASSERT_TRUE(hasSuffix("foobar", "bar")); + } + + /* ---------------------------------------------------------------------------- + * base64Encode + * --------------------------------------------------------------------------*/ + + TEST(base64Encode, emptyString) { + ASSERT_STREQ(base64Encode("").c_str(), ""); + } + + TEST(base64Encode, encodesAString) { + ASSERT_STREQ(base64Encode("quod erat demonstrandum").c_str(), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); + } + + TEST(base64Encode, encodeAndDecode) { + auto s = "quod erat demonstrandum"; + auto encoded = base64Encode(s); + auto decoded = base64Decode(encoded); + + ASSERT_STREQ(decoded.c_str(), s); + } + + /* ---------------------------------------------------------------------------- + * base64Decode + * --------------------------------------------------------------------------*/ + + TEST(base64Decode, emptyString) { + ASSERT_STREQ(base64Decode("").c_str(), ""); + } + + TEST(base64Decode, decodeAString) { + ASSERT_STREQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0=").c_str(), "quod erat demonstrandum"); + } + + /* ---------------------------------------------------------------------------- + * toLower + * --------------------------------------------------------------------------*/ + + TEST(toLower, emptyString) { + ASSERT_STREQ(toLower("").c_str(), ""); + } + + TEST(toLower, nonLetters) { + auto s = "!@(*$#)(@#=\\234_"; + ASSERT_STREQ(toLower(s).c_str(), s); + } + + // XXX: std::tolower() doesn't cover this. This probably doesn't matter + // since the context is paths and store paths specifically where such + // characters are not allowed. + TEST(toLower, DISABLED_umlauts) { + auto s = "ÄÖÜ"; + ASSERT_STREQ(toLower(s).c_str(), "äöü"); + } + + /* ---------------------------------------------------------------------------- + * string2Float + * --------------------------------------------------------------------------*/ + + TEST(string2Float, emptyString) { + double n; + ASSERT_EQ(string2Float("", n), false); + } + + TEST(string2Float, trivialConversions) { + double n; + ASSERT_EQ(string2Float("1.0", n), true); + ASSERT_EQ(n, 1.0); + + ASSERT_EQ(string2Float("0.0", n), true); + ASSERT_EQ(n, 0.0); + + ASSERT_EQ(string2Float("-100.25", n), true); + ASSERT_EQ(n, (-100.25)); + } + + /* ---------------------------------------------------------------------------- + * string2Int + * --------------------------------------------------------------------------*/ + + TEST(string2Int, emptyString) { + double n; + ASSERT_EQ(string2Int("", n), false); + } + + TEST(string2Int, trivialConversions) { + double n; + ASSERT_EQ(string2Int("1", n), true); + ASSERT_EQ(n, 1); + + ASSERT_EQ(string2Int("0", n), true); + ASSERT_EQ(n, 0); + + ASSERT_EQ(string2Int("-100", n), true); + ASSERT_EQ(n, (-100)); + } + + /* ---------------------------------------------------------------------------- + * statusOk + * --------------------------------------------------------------------------*/ + + TEST(statusOk, zeroIsOk) { + ASSERT_EQ(statusOk(0), true); + ASSERT_EQ(statusOk(1), false); + } + + /* ---------------------------------------------------------------------------- + * replaceInSet : XXX: this function isn't called anywhere! + * --------------------------------------------------------------------------*/ + + + /* ---------------------------------------------------------------------------- + * rewriteStrings + * --------------------------------------------------------------------------*/ + + TEST(rewriteStrings, emptyString) { + StringMap rewrites; + rewrites["this"] = "that"; + + ASSERT_STREQ(rewriteStrings("", rewrites).c_str(), ""); + } + + TEST(rewriteStrings, emptyRewrites) { + StringMap rewrites; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + } + + TEST(rewriteStrings, successfulRewrite) { + StringMap rewrites; + rewrites["this"] = "that"; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "that and that"); + } + + TEST(rewriteStrings, doesntOccur) { + StringMap rewrites; + rewrites["foo"] = "bar"; + + ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + } + + /* ---------------------------------------------------------------------------- + * replaceStrings + * --------------------------------------------------------------------------*/ + + TEST(replaceStrings, emptyString) { + ASSERT_STREQ(replaceStrings("", "this", "that").c_str(), ""); + ASSERT_STREQ(replaceStrings("this and that", "", "").c_str(), "this and that"); + } + + TEST(replaceStrings, successfulReplace) { + ASSERT_STREQ(replaceStrings("this and that", "this", "that").c_str(), "that and that"); + } + + TEST(replaceStrings, doesntOccur) { + ASSERT_STREQ(replaceStrings("this and that", "foo", "bar").c_str(), "this and that"); + } + + /* ---------------------------------------------------------------------------- + * trim + * --------------------------------------------------------------------------*/ + + TEST(trim, emptyString) { + ASSERT_STREQ(trim("").c_str(), ""); + } + + TEST(trim, removesWhitespace) { + ASSERT_STREQ(trim("foo").c_str(), "foo"); + ASSERT_STREQ(trim(" foo ").c_str(), "foo"); + ASSERT_STREQ(trim(" foo bar baz").c_str(), "foo bar baz"); + ASSERT_STREQ(trim(" \t foo bar baz\n").c_str(), "foo bar baz"); + } + + /* ---------------------------------------------------------------------------- + * chomp + * --------------------------------------------------------------------------*/ + + TEST(chomp, emptyString) { + ASSERT_STREQ(chomp("").c_str(), ""); + } + + TEST(chomp, removesWhitespace) { + ASSERT_STREQ(chomp("foo").c_str(), "foo"); + ASSERT_STREQ(chomp("foo ").c_str(), "foo"); + ASSERT_STREQ(chomp(" foo ").c_str(), " foo"); + ASSERT_STREQ(chomp(" foo bar baz ").c_str(), " foo bar baz"); + ASSERT_STREQ(chomp("\t foo bar baz\n").c_str(), "\t foo bar baz"); + } + + /* ---------------------------------------------------------------------------- + * quoteStrings + * --------------------------------------------------------------------------*/ + + TEST(quoteStrings, empty) { + Strings s = { }; + Strings expected = { }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + TEST(quoteStrings, emptyStrings) { + Strings s = { "", "", "" }; + Strings expected = { "''", "''", "''" }; + ASSERT_EQ(quoteStrings(s), expected); + + } + + TEST(quoteStrings, trivialQuote) { + Strings s = { "foo", "bar", "baz" }; + Strings expected = { "'foo'", "'bar'", "'baz'" }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + TEST(quoteStrings, quotedStrings) { + Strings s = { "'foo'", "'bar'", "'baz'" }; + Strings expected = { "''foo''", "''bar''", "''baz''" }; + + ASSERT_EQ(quoteStrings(s), expected); + } + + /* ---------------------------------------------------------------------------- + * tokenizeString + * --------------------------------------------------------------------------*/ + + TEST(tokenizeString, empty) { + auto s = ""; + Strings expected = { }; + + ASSERT_EQ(tokenizeString(""), expected); + } + + TEST(tokenizeString, tokenizeSpacesWithDefaults) { + auto s = "foo bar baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsWithDefaults) { + auto s = "foo\tbar\tbaz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesWithDefaults) { + auto s = "foo\t bar\t baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesNewlineWithDefaults) { + auto s = "foo\t\n bar\t\n baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + } + + TEST(tokenizeString, tokenizeTabsSpacesNewlineRetWithDefaults) { + auto s = "foo\t\n\r bar\t\n\r baz"; + Strings expected = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s), expected); + + auto s2 = "foo \t\n\r bar \t\n\r baz"; + Strings expected2 = { "foo", "bar", "baz" }; + + ASSERT_EQ(tokenizeString(s2), expected2); + } + + TEST(tokenizeString, tokenizeWithCustomSep) { + auto s = "foo\n,bar\n,baz\n"; + Strings expected = { "foo\n", "bar\n", "baz\n" }; + + ASSERT_EQ(tokenizeString(s, ","), expected); + } + + /* ---------------------------------------------------------------------------- + * get + * --------------------------------------------------------------------------*/ + + TEST(get, emptyContainer) { + StringMap s = { }; + auto expected = std::nullopt; + + ASSERT_EQ(get(s, "one"), expected); + } + + TEST(get, getFromContainer) { + StringMap s; + s["one"] = "yi"; + s["two"] = "er"; + auto expected = "yi"; + + ASSERT_EQ(get(s, "one"), expected); + } + + /* ---------------------------------------------------------------------------- + * filterANSIEscapes + * --------------------------------------------------------------------------*/ + + TEST(filterANSIEscapes, emptyString) { + auto s = ""; + auto expected = ""; + + ASSERT_STREQ(filterANSIEscapes(s).c_str(), expected); + } + + TEST(filterANSIEscapes, doesntChangePrintableChars) { + auto s = "09 2q304ruyhr slk2-19024 kjsadh sar f"; + + ASSERT_STREQ(filterANSIEscapes(s).c_str(), s); + } + + TEST(filterANSIEscapes, filtersColorCodes) { + auto s = "\u001b[30m A \u001b[31m B \u001b[32m C \u001b[33m D \u001b[0m"; + + ASSERT_STREQ(filterANSIEscapes(s, true, 2).c_str(), " A" ); + ASSERT_STREQ(filterANSIEscapes(s, true, 3).c_str(), " A " ); + ASSERT_STREQ(filterANSIEscapes(s, true, 4).c_str(), " A " ); + ASSERT_STREQ(filterANSIEscapes(s, true, 5).c_str(), " A B" ); + ASSERT_STREQ(filterANSIEscapes(s, true, 8).c_str(), " A B C" ); + } + + TEST(filterANSIEscapes, expandsTabs) { + auto s = "foo\tbar\tbaz"; + + ASSERT_STREQ(filterANSIEscapes(s, true).c_str(), "foo bar baz" ); + } + +} From 987b3d6469f03bff59614bd3b9097df858d07263 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 18:10:07 +0200 Subject: [PATCH 02/12] Use ASSERT_EQ instead of ASSERT_STREQ No need to use `c_str()` in combination with `ASSERT_STREQ`. It's possible to just use ASSERT_EQ on std::string --- tests/unit-tests/test-util.cc | 108 +++++++++++++++++----------------- 1 file changed, 54 insertions(+), 54 deletions(-) diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index 05d8a0039d9..62ca8f11ccf 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -12,14 +12,14 @@ namespace nix { TEST(absPath, doesntChangeRoot) { auto p = absPath("/"); - ASSERT_STREQ(p.c_str(), "/"); + ASSERT_EQ(p, "/"); } TEST(absPath, turnsEmptyPathIntoCWD) { char cwd[PATH_MAX+1]; auto p = absPath(""); - ASSERT_STREQ(p.c_str(), getcwd((char*)&cwd, PATH_MAX)); + ASSERT_EQ(p, getcwd((char*)&cwd, PATH_MAX)); } TEST(absPath, usesOptionalBasePathWhenGiven) { @@ -28,7 +28,7 @@ namespace nix { auto p = absPath("", cwd); - ASSERT_STREQ(p.c_str(), cwd); + ASSERT_EQ(p, cwd); } TEST(absPath, isIdempotent) { @@ -37,7 +37,7 @@ namespace nix { auto p1 = absPath(cwd); auto p2 = absPath(p1); - ASSERT_STREQ(p1.c_str(), p2.c_str()); + ASSERT_EQ(p1, p2); } @@ -46,8 +46,8 @@ namespace nix { auto p1 = absPath(path); auto p2 = absPath(p1); - ASSERT_STREQ(p1.c_str(), "/some/path/with/trailing/dot"); - ASSERT_STREQ(p1.c_str(), p2.c_str()); + ASSERT_EQ(p1, "/some/path/with/trailing/dot"); + ASSERT_EQ(p1, p2); } /* ---------------------------------------------------------------------------- @@ -58,21 +58,21 @@ namespace nix { auto path = "/this/is/a/path//"; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, removesDots) { auto path = "/this/./is/a/path/./"; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, removesDots2) { auto path = "/this/a/../is/a////path/foo/.."; auto p = canonPath(path); - ASSERT_STREQ(p.c_str(), "/this/is/a/path"); + ASSERT_EQ(p, "/this/is/a/path"); } TEST(canonPath, requiresAbsolutePath) { @@ -91,18 +91,18 @@ namespace nix { TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { auto p = dirOf("/"); - ASSERT_STREQ(p.c_str(), ""); + ASSERT_EQ(p, ""); } TEST(dirOf, returnsFirstPathComponent) { auto p1 = dirOf("/dir/"); - ASSERT_STREQ(p1.c_str(), "/dir"); + ASSERT_EQ(p1, "/dir"); auto p2 = dirOf("/dir"); - ASSERT_STREQ(p2.c_str(), "/"); + ASSERT_EQ(p2, "/"); auto p3 = dirOf("/dir/.."); - ASSERT_STREQ(p3.c_str(), "/dir"); + ASSERT_EQ(p3, "/dir"); auto p4 = dirOf("/dir/../"); - ASSERT_STREQ(p4.c_str(), "/dir/.."); + ASSERT_EQ(p4, "/dir/.."); } /* ---------------------------------------------------------------------------- @@ -111,29 +111,29 @@ namespace nix { TEST(baseNameOf, emptyPath) { auto p1 = baseNameOf(""); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } TEST(baseNameOf, pathOnRoot) { auto p1 = baseNameOf("/dir"); - ASSERT_STREQ(std::string(p1).c_str(), "dir"); + ASSERT_EQ(std::string(p1), "dir"); } TEST(baseNameOf, relativePath) { auto p1 = baseNameOf("dir/foo"); - ASSERT_STREQ(std::string(p1).c_str(), "foo"); + ASSERT_EQ(std::string(p1), "foo"); } TEST(baseNameOf, pathWithTrailingSlashRoot) { auto p1 = baseNameOf("/"); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return // "" but it actually returns "dir" TEST(baseNameOf, DISABLED_trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_STREQ(std::string(p1).c_str(), ""); + ASSERT_EQ(std::string(p1), ""); } /* ---------------------------------------------------------------------------- @@ -265,11 +265,11 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(base64Encode, emptyString) { - ASSERT_STREQ(base64Encode("").c_str(), ""); + ASSERT_EQ(base64Encode(""), ""); } TEST(base64Encode, encodesAString) { - ASSERT_STREQ(base64Encode("quod erat demonstrandum").c_str(), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); + ASSERT_EQ(base64Encode("quod erat demonstrandum"), "cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="); } TEST(base64Encode, encodeAndDecode) { @@ -277,7 +277,7 @@ namespace nix { auto encoded = base64Encode(s); auto decoded = base64Decode(encoded); - ASSERT_STREQ(decoded.c_str(), s); + ASSERT_EQ(decoded, s); } /* ---------------------------------------------------------------------------- @@ -285,11 +285,11 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(base64Decode, emptyString) { - ASSERT_STREQ(base64Decode("").c_str(), ""); + ASSERT_EQ(base64Decode(""), ""); } TEST(base64Decode, decodeAString) { - ASSERT_STREQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0=").c_str(), "quod erat demonstrandum"); + ASSERT_EQ(base64Decode("cXVvZCBlcmF0IGRlbW9uc3RyYW5kdW0="), "quod erat demonstrandum"); } /* ---------------------------------------------------------------------------- @@ -297,12 +297,12 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(toLower, emptyString) { - ASSERT_STREQ(toLower("").c_str(), ""); + ASSERT_EQ(toLower(""), ""); } TEST(toLower, nonLetters) { auto s = "!@(*$#)(@#=\\234_"; - ASSERT_STREQ(toLower(s).c_str(), s); + ASSERT_EQ(toLower(s), s); } // XXX: std::tolower() doesn't cover this. This probably doesn't matter @@ -310,7 +310,7 @@ namespace nix { // characters are not allowed. TEST(toLower, DISABLED_umlauts) { auto s = "ÄÖÜ"; - ASSERT_STREQ(toLower(s).c_str(), "äöü"); + ASSERT_EQ(toLower(s), "äöü"); } /* ---------------------------------------------------------------------------- @@ -377,27 +377,27 @@ namespace nix { StringMap rewrites; rewrites["this"] = "that"; - ASSERT_STREQ(rewriteStrings("", rewrites).c_str(), ""); + ASSERT_EQ(rewriteStrings("", rewrites), ""); } TEST(rewriteStrings, emptyRewrites) { StringMap rewrites; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "this and that"); } TEST(rewriteStrings, successfulRewrite) { StringMap rewrites; rewrites["this"] = "that"; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "that and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "that and that"); } TEST(rewriteStrings, doesntOccur) { StringMap rewrites; rewrites["foo"] = "bar"; - ASSERT_STREQ(rewriteStrings("this and that", rewrites).c_str(), "this and that"); + ASSERT_EQ(rewriteStrings("this and that", rewrites), "this and that"); } /* ---------------------------------------------------------------------------- @@ -405,16 +405,16 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(replaceStrings, emptyString) { - ASSERT_STREQ(replaceStrings("", "this", "that").c_str(), ""); - ASSERT_STREQ(replaceStrings("this and that", "", "").c_str(), "this and that"); + ASSERT_EQ(replaceStrings("", "this", "that"), ""); + ASSERT_EQ(replaceStrings("this and that", "", ""), "this and that"); } TEST(replaceStrings, successfulReplace) { - ASSERT_STREQ(replaceStrings("this and that", "this", "that").c_str(), "that and that"); + ASSERT_EQ(replaceStrings("this and that", "this", "that"), "that and that"); } TEST(replaceStrings, doesntOccur) { - ASSERT_STREQ(replaceStrings("this and that", "foo", "bar").c_str(), "this and that"); + ASSERT_EQ(replaceStrings("this and that", "foo", "bar"), "this and that"); } /* ---------------------------------------------------------------------------- @@ -422,14 +422,14 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(trim, emptyString) { - ASSERT_STREQ(trim("").c_str(), ""); + ASSERT_EQ(trim(""), ""); } TEST(trim, removesWhitespace) { - ASSERT_STREQ(trim("foo").c_str(), "foo"); - ASSERT_STREQ(trim(" foo ").c_str(), "foo"); - ASSERT_STREQ(trim(" foo bar baz").c_str(), "foo bar baz"); - ASSERT_STREQ(trim(" \t foo bar baz\n").c_str(), "foo bar baz"); + ASSERT_EQ(trim("foo"), "foo"); + ASSERT_EQ(trim(" foo "), "foo"); + ASSERT_EQ(trim(" foo bar baz"), "foo bar baz"); + ASSERT_EQ(trim(" \t foo bar baz\n"), "foo bar baz"); } /* ---------------------------------------------------------------------------- @@ -437,15 +437,15 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(chomp, emptyString) { - ASSERT_STREQ(chomp("").c_str(), ""); + ASSERT_EQ(chomp(""), ""); } TEST(chomp, removesWhitespace) { - ASSERT_STREQ(chomp("foo").c_str(), "foo"); - ASSERT_STREQ(chomp("foo ").c_str(), "foo"); - ASSERT_STREQ(chomp(" foo ").c_str(), " foo"); - ASSERT_STREQ(chomp(" foo bar baz ").c_str(), " foo bar baz"); - ASSERT_STREQ(chomp("\t foo bar baz\n").c_str(), "\t foo bar baz"); + ASSERT_EQ(chomp("foo"), "foo"); + ASSERT_EQ(chomp("foo "), "foo"); + ASSERT_EQ(chomp(" foo "), " foo"); + ASSERT_EQ(chomp(" foo bar baz "), " foo bar baz"); + ASSERT_EQ(chomp("\t foo bar baz\n"), "\t foo bar baz"); } /* ---------------------------------------------------------------------------- @@ -566,29 +566,29 @@ namespace nix { auto s = ""; auto expected = ""; - ASSERT_STREQ(filterANSIEscapes(s).c_str(), expected); + ASSERT_EQ(filterANSIEscapes(s), expected); } TEST(filterANSIEscapes, doesntChangePrintableChars) { auto s = "09 2q304ruyhr slk2-19024 kjsadh sar f"; - ASSERT_STREQ(filterANSIEscapes(s).c_str(), s); + ASSERT_EQ(filterANSIEscapes(s), s); } TEST(filterANSIEscapes, filtersColorCodes) { auto s = "\u001b[30m A \u001b[31m B \u001b[32m C \u001b[33m D \u001b[0m"; - ASSERT_STREQ(filterANSIEscapes(s, true, 2).c_str(), " A" ); - ASSERT_STREQ(filterANSIEscapes(s, true, 3).c_str(), " A " ); - ASSERT_STREQ(filterANSIEscapes(s, true, 4).c_str(), " A " ); - ASSERT_STREQ(filterANSIEscapes(s, true, 5).c_str(), " A B" ); - ASSERT_STREQ(filterANSIEscapes(s, true, 8).c_str(), " A B C" ); + ASSERT_EQ(filterANSIEscapes(s, true, 2), " A" ); + ASSERT_EQ(filterANSIEscapes(s, true, 3), " A " ); + ASSERT_EQ(filterANSIEscapes(s, true, 4), " A " ); + ASSERT_EQ(filterANSIEscapes(s, true, 5), " A B" ); + ASSERT_EQ(filterANSIEscapes(s, true, 8), " A B C" ); } TEST(filterANSIEscapes, expandsTabs) { auto s = "foo\tbar\tbaz"; - ASSERT_STREQ(filterANSIEscapes(s, true).c_str(), "foo bar baz" ); + ASSERT_EQ(filterANSIEscapes(s, true), "foo bar baz" ); } } From 1f3602a2c9c3bfeea7cc8ee4d478c77dec349206 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 18:15:13 +0200 Subject: [PATCH 03/12] Remove replaceInSet The function isn't being used anywhere so it seems safe to remove --- src/libutil/util.hh | 11 ----------- tests/unit-tests/test-util.cc | 4 ---- 2 files changed, 15 deletions(-) diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 1b263abccbb..8770add64a4 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -389,17 +389,6 @@ string replaceStrings(const std::string & s, std::string rewriteStrings(const std::string & s, const StringMap & rewrites); -/* If a set contains 'from', remove it and insert 'to'. */ -template -void replaceInSet(std::set & set, const T & from, const T & to) -{ - auto i = set.find(from); - if (i == set.end()) return; - set.erase(i); - set.insert(to); -} - - /* Convert the exit status of a child as returned by wait() into an error string. */ string statusToString(int status); diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index 62ca8f11ccf..e0738614e79 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -364,10 +364,6 @@ namespace nix { ASSERT_EQ(statusOk(1), false); } - /* ---------------------------------------------------------------------------- - * replaceInSet : XXX: this function isn't called anywhere! - * --------------------------------------------------------------------------*/ - /* ---------------------------------------------------------------------------- * rewriteStrings From 73d0b5d807edee05910cff4a30549393d5375d89 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Thu, 7 May 2020 19:29:10 +0200 Subject: [PATCH 04/12] Drop unnecessary std::string --- tests/unit-tests/test-util.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/unit-tests/test-util.cc b/tests/unit-tests/test-util.cc index e0738614e79..a0fa7c10989 100644 --- a/tests/unit-tests/test-util.cc +++ b/tests/unit-tests/test-util.cc @@ -111,29 +111,29 @@ namespace nix { TEST(baseNameOf, emptyPath) { auto p1 = baseNameOf(""); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } TEST(baseNameOf, pathOnRoot) { auto p1 = baseNameOf("/dir"); - ASSERT_EQ(std::string(p1), "dir"); + ASSERT_EQ(p1, "dir"); } TEST(baseNameOf, relativePath) { auto p1 = baseNameOf("dir/foo"); - ASSERT_EQ(std::string(p1), "foo"); + ASSERT_EQ(p1, "foo"); } TEST(baseNameOf, pathWithTrailingSlashRoot) { auto p1 = baseNameOf("/"); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return // "" but it actually returns "dir" TEST(baseNameOf, DISABLED_trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_EQ(std::string(p1), ""); + ASSERT_EQ(p1, ""); } /* ---------------------------------------------------------------------------- From 7cc7cef9502dc388706561cbdf275e070520e65d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:34:09 +0200 Subject: [PATCH 05/12] Move unit tests to sr/libutil/tests, use mk make rules --- Makefile | 4 ++-- src/libutil/tests/local.mk | 11 +++++++++++ .../test-util.cc => src/libutil/tests/tests.cc | 4 ++-- tests/unit-tests/local.mk | 4 ---- 4 files changed, 15 insertions(+), 8 deletions(-) create mode 100644 src/libutil/tests/local.mk rename tests/unit-tests/test-util.cc => src/libutil/tests/tests.cc (99%) delete mode 100644 tests/unit-tests/local.mk diff --git a/Makefile b/Makefile index 06de35a2709..0ba011e2ad0 100644 --- a/Makefile +++ b/Makefile @@ -3,6 +3,7 @@ makefiles = \ local.mk \ nix-rust/local.mk \ src/libutil/local.mk \ + src/libutil/tests/local.mk \ src/libstore/local.mk \ src/libfetchers/local.mk \ src/libmain/local.mk \ @@ -16,8 +17,7 @@ makefiles = \ misc/upstart/local.mk \ doc/manual/local.mk \ tests/local.mk \ - tests/plugins/local.mk \ - tests/unit-tests/local.mk + tests/plugins/local.mk -include Makefile.config diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk new file mode 100644 index 00000000000..9c2d2a7bbc8 --- /dev/null +++ b/src/libutil/tests/local.mk @@ -0,0 +1,11 @@ +programs += libutil-tests + +libutil-tests_DIR := $(d) + +libutil-tests_SOURCES := $(wildcard $(d)/*.cc) + +libutil-tests_CXXFLAGS += -I src/libutil + +libutil-tests_LIBS = libutil + +libutil-tests_LDFLAGS := $$(pkg-config --libs gtest_main) diff --git a/tests/unit-tests/test-util.cc b/src/libutil/tests/tests.cc similarity index 99% rename from tests/unit-tests/test-util.cc rename to src/libutil/tests/tests.cc index a0fa7c10989..fb593b5e654 100644 --- a/tests/unit-tests/test-util.cc +++ b/src/libutil/tests/tests.cc @@ -1,5 +1,5 @@ -#include "../../src/libutil/util.hh" -#include "../../src/libutil/types.hh" +#include "util.hh" +#include "types.hh" #include diff --git a/tests/unit-tests/local.mk b/tests/unit-tests/local.mk deleted file mode 100644 index ed11261c1a0..00000000000 --- a/tests/unit-tests/local.mk +++ /dev/null @@ -1,4 +0,0 @@ -LIBS=-larchive -lcrypto -llzma -lbz2 -lz -lbrotlienc -lbrotlidec -unit-tests: - echo $(nix_LDFLAGS) - $(CXX) -o unit-test $(nix_CXXFLAGS) $(nix_LDFLAGS) $(LIBS) --std=c++17 $$(pkg-config --libs gtest_main) ./src/libutil/*.o tests/unit-tests/test-util.cc From 72b9d971bc4ff5b5dcb9349d03526a5ef3e4300e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:35:57 +0200 Subject: [PATCH 06/12] Fix warning --- src/libutil/tests/tests.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index fb593b5e654..0fb4411e834 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -481,8 +481,7 @@ namespace nix { * --------------------------------------------------------------------------*/ TEST(tokenizeString, empty) { - auto s = ""; - Strings expected = { }; + Strings expected = { }; ASSERT_EQ(tokenizeString(""), expected); } From 7898cdb75a721cc21867ccdbcc523e95f2db0354 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 11:49:40 +0200 Subject: [PATCH 07/12] make check: Run unit tests --- mk/programs.mk | 6 ++++++ mk/tracing.mk | 1 + nix-rust/local.mk | 2 +- src/libutil/tests/local.mk | 2 ++ 4 files changed, 10 insertions(+), 1 deletion(-) diff --git a/mk/programs.mk b/mk/programs.mk index d93df446808..2d0c988d4cd 100644 --- a/mk/programs.mk +++ b/mk/programs.mk @@ -76,4 +76,10 @@ define build-program programs-list += $$($(1)_PATH) clean-files += $$($(1)_PATH) $$(_d)/*.o $$(_d)/.*.dep $$($(1)_DEPS) $$($(1)_OBJS) dist-files += $$(_srcs) + + # Phony target to run this program (typically as a dependency of 'check'). + .PHONY: $(1)_RUN + $(1)_RUN: $$($(1)_PATH) + $(trace-test) $$($(1)_PATH) + endef diff --git a/mk/tracing.mk b/mk/tracing.mk index 13912d3c782..54c77ab6075 100644 --- a/mk/tracing.mk +++ b/mk/tracing.mk @@ -11,6 +11,7 @@ ifeq ($(V), 0) trace-javac = @echo " JAVAC " $@; trace-jar = @echo " JAR " $@; trace-mkdir = @echo " MKDIR " $@; + trace-test = @echo " TEST " $@; suppress = @ diff --git a/nix-rust/local.mk b/nix-rust/local.mk index 1e006e500c8..e4bfde31b8b 100644 --- a/nix-rust/local.mk +++ b/nix-rust/local.mk @@ -41,5 +41,5 @@ ifneq ($(OS), Darwin) check: rust-tests rust-tests: - cd nix-rust && CARGO_HOME=$$(if [[ -d vendor ]]; then echo vendor; fi) cargo test --release $$(if [[ -d vendor ]]; then echo --offline; fi) + $(trace-test) cd nix-rust && CARGO_HOME=$$(if [[ -d vendor ]]; then echo vendor; fi) cargo test --release $$(if [[ -d vendor ]]; then echo --offline; fi) endif diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index 9c2d2a7bbc8..a188d5093a4 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -1,3 +1,5 @@ +check: libutil-tests_RUN + programs += libutil-tests libutil-tests_DIR := $(d) From ca657525b88e788b72ce53ecc6d140a573476644 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 12:03:27 +0200 Subject: [PATCH 08/12] Don't install unit tests --- mk/programs.mk | 20 ++++++++++++-------- src/libutil/tests/local.mk | 2 ++ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/mk/programs.mk b/mk/programs.mk index 2d0c988d4cd..3fa9685c39d 100644 --- a/mk/programs.mk +++ b/mk/programs.mk @@ -35,24 +35,28 @@ define build-program $$(trace-ld) $(CXX) -o $$@ $$(LDFLAGS) $$(GLOBAL_LDFLAGS) $$($(1)_OBJS) $$($(1)_LDFLAGS) $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_LDFLAGS_USE)) $(1)_INSTALL_DIR ?= $$(bindir) - $(1)_INSTALL_PATH := $$($(1)_INSTALL_DIR)/$(1) - $$(eval $$(call create-dir, $$($(1)_INSTALL_DIR))) + ifdef $(1)_INSTALL_DIR - install: $(DESTDIR)$$($(1)_INSTALL_PATH) + $(1)_INSTALL_PATH := $$($(1)_INSTALL_DIR)/$(1) - ifeq ($(BUILD_SHARED_LIBS), 1) + $$(eval $$(call create-dir, $$($(1)_INSTALL_DIR))) - _libs_final := $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_INSTALL_PATH)) + install: $(DESTDIR)$$($(1)_INSTALL_PATH) - $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_OBJS) $$(_libs_final) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ + ifeq ($(BUILD_SHARED_LIBS), 1) + + _libs_final := $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_INSTALL_PATH)) + + $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_OBJS) $$(_libs_final) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ $$(trace-ld) $(CXX) -o $$@ $$(LDFLAGS) $$(GLOBAL_LDFLAGS) $$($(1)_OBJS) $$($(1)_LDFLAGS) $$(foreach lib, $$($(1)_LIBS), $$($$(lib)_LDFLAGS_USE_INSTALLED)) - else + else - $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_PATH) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ + $(DESTDIR)$$($(1)_INSTALL_PATH): $$($(1)_PATH) | $(DESTDIR)$$($(1)_INSTALL_DIR)/ install -t $(DESTDIR)$$($(1)_INSTALL_DIR) $$< + endif endif # Propagate CFLAGS and CXXFLAGS to the individual object files. diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index a188d5093a4..5f4b3edc5d1 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -4,6 +4,8 @@ programs += libutil-tests libutil-tests_DIR := $(d) +libutil-tests_INSTALL_DIR := + libutil-tests_SOURCES := $(wildcard $(d)/*.cc) libutil-tests_CXXFLAGS += -I src/libutil From 5b8883faac0cff928a4359b4260b5be09a311d4b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 8 May 2020 12:09:00 +0200 Subject: [PATCH 09/12] configure: Look for gtest --- Makefile.config.in | 19 ++++++++++--------- configure.ac | 4 ++++ src/libutil/tests/local.mk | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/Makefile.config.in b/Makefile.config.in index e7a12089a86..b632444e8ab 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -1,36 +1,38 @@ AR = @AR@ BDW_GC_LIBS = @BDW_GC_LIBS@ +BOOST_LDFLAGS = @BOOST_LDFLAGS@ BUILD_SHARED_LIBS = @BUILD_SHARED_LIBS@ CC = @CC@ CFLAGS = @CFLAGS@ CXX = @CXX@ CXXFLAGS = @CXXFLAGS@ -LDFLAGS = @LDFLAGS@ +EDITLINE_LIBS = @EDITLINE_LIBS@ ENABLE_S3 = @ENABLE_S3@ -HAVE_SODIUM = @HAVE_SODIUM@ +GTEST_LIBS = @GTEST_LIBS@ HAVE_SECCOMP = @HAVE_SECCOMP@ -BOOST_LDFLAGS = @BOOST_LDFLAGS@ +HAVE_SODIUM = @HAVE_SODIUM@ +LDFLAGS = @LDFLAGS@ +LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ +LIBBROTLI_LIBS = @LIBBROTLI_LIBS@ LIBCURL_LIBS = @LIBCURL_LIBS@ +LIBLZMA_LIBS = @LIBLZMA_LIBS@ OPENSSL_LIBS = @OPENSSL_LIBS@ PACKAGE_NAME = @PACKAGE_NAME@ PACKAGE_VERSION = @PACKAGE_VERSION@ SODIUM_LIBS = @SODIUM_LIBS@ -LIBLZMA_LIBS = @LIBLZMA_LIBS@ SQLITE3_LIBS = @SQLITE3_LIBS@ -LIBBROTLI_LIBS = @LIBBROTLI_LIBS@ -LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ -EDITLINE_LIBS = @EDITLINE_LIBS@ bash = @bash@ bindir = @bindir@ -lsof = @lsof@ datadir = @datadir@ datarootdir = @datarootdir@ +doc_generate = @doc_generate@ docdir = @docdir@ exec_prefix = @exec_prefix@ includedir = @includedir@ libdir = @libdir@ libexecdir = @libexecdir@ localstatedir = @localstatedir@ +lsof = @lsof@ mandir = @mandir@ pkglibdir = $(libdir)/$(PACKAGE_NAME) prefix = @prefix@ @@ -38,6 +40,5 @@ sandbox_shell = @sandbox_shell@ storedir = @storedir@ sysconfdir = @sysconfdir@ system = @system@ -doc_generate = @doc_generate@ xmllint = @xmllint@ xsltproc = @xsltproc@ diff --git a/configure.ac b/configure.ac index b868343d251..c3007b4b6b4 100644 --- a/configure.ac +++ b/configure.ac @@ -266,6 +266,10 @@ if test "$gc" = yes; then fi +# Look for gtest. +PKG_CHECK_MODULES([GTEST], [gtest_main]) + + # documentation generation switch AC_ARG_ENABLE(doc-gen, AC_HELP_STRING([--disable-doc-gen], [disable documentation generation]), diff --git a/src/libutil/tests/local.mk b/src/libutil/tests/local.mk index 5f4b3edc5d1..a297edb6478 100644 --- a/src/libutil/tests/local.mk +++ b/src/libutil/tests/local.mk @@ -12,4 +12,4 @@ libutil-tests_CXXFLAGS += -I src/libutil libutil-tests_LIBS = libutil -libutil-tests_LDFLAGS := $$(pkg-config --libs gtest_main) +libutil-tests_LDFLAGS := $(GTEST_LIBS) From e3df9c2a6e867d0e038cb26af40501511607e007 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:03:44 +0200 Subject: [PATCH 10/12] Enable `dirOf` test Adjusted the doc comment for `dirOf` to reflect the implementation behavior. --- src/libutil/tests/tests.cc | 6 ++---- src/libutil/util.hh | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 0fb4411e834..1b016430da8 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -86,12 +86,10 @@ namespace nix { * dirOf * --------------------------------------------------------------------------*/ - // XXX: according to the doc of `dirOf`, dirOf("/") and dirOf("/foo") - // should both return "" but it actually returns "/" in both cases - TEST(dirOf, DISABLED_returnsEmptyStringForRoot) { + TEST(dirOf, returnsEmptyStringForRoot) { auto p = dirOf("/"); - ASSERT_EQ(p, ""); + ASSERT_EQ(p, "/"); } TEST(dirOf, returnsFirstPathComponent) { diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 8770add64a4..38a0f7a5cc6 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -58,8 +58,8 @@ Path canonPath(const Path & path, bool resolveSymlinks = false); /* Return the directory part of the given canonical path, i.e., everything before the final `/'. If the path is the root or an - immediate child thereof (e.g., `/foo'), this means an empty string - is returned. */ + immediate child thereof (e.g., `/foo'), this means `/' + is returned.*/ Path dirOf(const Path & path); /* Return the base name of the given canonical path, i.e., everything From 2191141274cfefcb9ffbafbbfcdc58414fb28f1a Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:07:40 +0200 Subject: [PATCH 11/12] Enable `baseNameOf` test Add note about removal of trailing slashes in the doc comment of baseNameOf and enabled the test. --- src/libutil/tests/tests.cc | 6 ++---- src/libutil/util.hh | 2 +- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index 1b016430da8..d46f6a5a4bc 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -127,11 +127,9 @@ namespace nix { ASSERT_EQ(p1, ""); } - // XXX: according to the doc of `baseNameOf`, baseNameOf("/dir/") should return - // "" but it actually returns "dir" - TEST(baseNameOf, DISABLED_trailingSlash) { + TEST(baseNameOf, trailingSlash) { auto p1 = baseNameOf("/dir/"); - ASSERT_EQ(p1, ""); + ASSERT_EQ(p1, "dir"); } /* ---------------------------------------------------------------------------- diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 38a0f7a5cc6..a63ee05b377 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -63,7 +63,7 @@ Path canonPath(const Path & path, bool resolveSymlinks = false); Path dirOf(const Path & path); /* Return the base name of the given canonical path, i.e., everything - following the final `/'. */ + following the final `/' (trailing slashes are removed). */ std::string_view baseNameOf(std::string_view path); /* Check whether 'path' is a descendant of 'dir'. */ From 181a47d8845e06edc9653e99de589376a1a96bb6 Mon Sep 17 00:00:00 2001 From: Tobias Pflug Date: Fri, 8 May 2020 15:13:55 +0200 Subject: [PATCH 12/12] Enable toLower umlauts test Update comment and enable the test --- src/libutil/tests/tests.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libutil/tests/tests.cc b/src/libutil/tests/tests.cc index d46f6a5a4bc..5abfe2c9c96 100644 --- a/src/libutil/tests/tests.cc +++ b/src/libutil/tests/tests.cc @@ -301,12 +301,12 @@ namespace nix { ASSERT_EQ(toLower(s), s); } - // XXX: std::tolower() doesn't cover this. This probably doesn't matter - // since the context is paths and store paths specifically where such - // characters are not allowed. - TEST(toLower, DISABLED_umlauts) { + // std::tolower() doesn't handle unicode characters. In the context of + // store paths this isn't relevant but doesn't hurt to record this behavior + // here. + TEST(toLower, umlauts) { auto s = "ÄÖÜ"; - ASSERT_EQ(toLower(s), "äöü"); + ASSERT_EQ(toLower(s), "ÄÖÜ"); } /* ----------------------------------------------------------------------------