Skip to content

Commit

Permalink
Add unit tests for libexpr (#5377)
Browse files Browse the repository at this point in the history
* libexpr: fix builtins.split example

The example was previously indicating that multiple whitespaces would be
collapsed into a single captured whitespace. That isn't true and was
likely a mistake when being documented initially.

* Fix segfault on unitilized list when printing value

Since lists are just chunks of memory the individual elements in the
list might be unitilized when a programming error happens within Nix.

In this case the values are null-initialized (at least with Boehm GC)
and we can avoid a nullptr deref when printing them.

I ran into this issue while ensuring that new expression tests would
show the actual value on an assertion failure.

This is unlikely to cause any runtime performance regressions as
printing values is not really in the hot path (unless the repl is the
primary use case).

* Add operator<< for ValueTypes

* Add libexpr tests

This introduces tests for libexpr that evalulate various trivial Nix
language expressions and primop invocations that should be good smoke
tests wheter or not the implementation is behaving as expected.
  • Loading branch information
andir committed May 6, 2022
1 parent b470218 commit 059ae7f
Show file tree
Hide file tree
Showing 10 changed files with 1,267 additions and 2 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ perl/Makefile.config
/src/libexpr/parser-tab.hh
/src/libexpr/parser-tab.output
/src/libexpr/nix.tbl
/src/libexpr/tests/libexpr-tests

# /src/libstore/
*.gen.*
Expand Down
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ makefiles = \
src/libfetchers/local.mk \
src/libmain/local.mk \
src/libexpr/local.mk \
src/libexpr/tests/local.mk \
src/libcmd/local.mk \
src/nix/local.mk \
src/resolve-system-dependencies/local.mk \
Expand Down
10 changes: 9 additions & 1 deletion src/libexpr/eval.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,10 @@ void Value::print(const SymbolTable & symbols, std::ostream & str,
else {
str << "[ ";
for (auto v2 : listItems()) {
v2->print(symbols, str, seen);
if (v2)
v2->print(symbols, str, seen);
else
str << "(nullptr)";
str << " ";
}
str << "]";
Expand Down Expand Up @@ -184,6 +187,11 @@ void Value::print(const SymbolTable & symbols, std::ostream & str, bool showRepe
print(symbols, str, showRepeated ? nullptr : &seen);
}

// Pretty print types for assertion errors
std::ostream & operator << (std::ostream & os, const ValueType t) {
os << showType(t);
return os;
}

std::string printValue(const EvalState & state, const Value & v)
{
Expand Down
1 change: 1 addition & 0 deletions src/libexpr/eval.hh
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ typedef std::map<Path, StorePath> SrcToStore;

std::ostream & printValue(const EvalState & state, std::ostream & str, const Value & v);
std::string printValue(const EvalState & state, const Value & v);
std::ostream & operator << (std::ostream & os, const ValueType t);


typedef std::pair<std::string, std::string> SearchPathElem;
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3632,7 +3632,7 @@ static RegisterPrimOp primop_split({
Evaluates to `[ "" [ "a" null ] "b" [ null "c" ] "" ]`.
```nix
builtins.split "([[:upper:]]+)" " FOO "
builtins.split "([[:upper:]]+)" " FOO "
```
Evaluates to `[ " " [ "FOO" ] " " ]`.
Expand Down
68 changes: 68 additions & 0 deletions src/libexpr/tests/json.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
#include "libexprtests.hh"
#include "value-to-json.hh"

namespace nix {
// Testing the conversion to JSON

class JSONValueTest : public LibExprTest {
protected:
std::string getJSONValue(Value& value) {
std::stringstream ss;
PathSet ps;
printValueAsJSON(state, true, value, noPos, ss, ps);
return ss.str();
}
};

TEST_F(JSONValueTest, null) {
Value v;
v.mkNull();
ASSERT_EQ(getJSONValue(v), "null");
}

TEST_F(JSONValueTest, BoolFalse) {
Value v;
v.mkBool(false);
ASSERT_EQ(getJSONValue(v),"false");
}

TEST_F(JSONValueTest, BoolTrue) {
Value v;
v.mkBool(true);
ASSERT_EQ(getJSONValue(v), "true");
}

TEST_F(JSONValueTest, IntPositive) {
Value v;
v.mkInt(100);
ASSERT_EQ(getJSONValue(v), "100");
}

TEST_F(JSONValueTest, IntNegative) {
Value v;
v.mkInt(-100);
ASSERT_EQ(getJSONValue(v), "-100");
}

TEST_F(JSONValueTest, String) {
Value v;
v.mkString("test");
ASSERT_EQ(getJSONValue(v), "\"test\"");
}

TEST_F(JSONValueTest, StringQuotes) {
Value v;

v.mkString("test\"");
ASSERT_EQ(getJSONValue(v), "\"test\\\"\"");
}

// The dummy store doesn't support writing files. Fails with this exception message:
// C++ exception with description "error: operation 'addToStoreFromDump' is
// not supported by store 'dummy'" thrown in the test body.
TEST_F(JSONValueTest, DISABLED_Path) {
Value v;
v.mkPath("test");
ASSERT_EQ(getJSONValue(v), "\"/nix/store/g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-x\"");
}
} /* namespace nix */
136 changes: 136 additions & 0 deletions src/libexpr/tests/libexprtests.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
#include <gtest/gtest.h>
#include <gmock/gmock.h>

#include "value.hh"
#include "nixexpr.hh"
#include "eval.hh"
#include "eval-inline.hh"
#include "store-api.hh"


namespace nix {
class LibExprTest : public ::testing::Test {
public:
static void SetUpTestSuite() {
initGC();
}

protected:
LibExprTest()
: store(openStore("dummy:https://"))
, state({}, store)
{
}
Value eval(std::string input, bool forceValue = true) {
Value v;
Expr * e = state.parseExprFromString(input, "");
assert(e);
state.eval(e, v);
if (forceValue)
state.forceValue(v, noPos);
return v;
}

Symbol createSymbol(const char * value) {
return state.symbols.create(value);
}

ref<Store> store;
EvalState state;
};

MATCHER(IsListType, "") {
return arg != nList;
}

MATCHER(IsList, "") {
return arg.type() == nList;
}

MATCHER(IsString, "") {
return arg.type() == nString;
}

MATCHER(IsNull, "") {
return arg.type() == nNull;
}

MATCHER(IsThunk, "") {
return arg.type() == nThunk;
}

MATCHER(IsAttrs, "") {
return arg.type() == nAttrs;
}

MATCHER_P(IsStringEq, s, fmt("The string is equal to \"%1%\"", s)) {
if (arg.type() != nString) {
return false;
}
return std::string_view(arg.string.s) == s;
}

MATCHER_P(IsIntEq, v, fmt("The string is equal to \"%1%\"", v)) {
if (arg.type() != nInt) {
return false;
}
return arg.integer == v;
}

MATCHER_P(IsFloatEq, v, fmt("The float is equal to \"%1%\"", v)) {
if (arg.type() != nFloat) {
return false;
}
return arg.fpoint == v;
}

MATCHER(IsTrue, "") {
if (arg.type() != nBool) {
return false;
}
return arg.boolean == true;
}

MATCHER(IsFalse, "") {
if (arg.type() != nBool) {
return false;
}
return arg.boolean == false;
}

MATCHER_P(IsPathEq, p, fmt("Is a path equal to \"%1%\"", p)) {
if (arg.type() != nPath) {
*result_listener << "Expected a path got " << arg.type();
return false;
} else if (std::string_view(arg.string.s) != p) {
*result_listener << "Expected a path that equals \"" << p << "\" but got: " << arg.string.s;
return false;
}
return true;
}


MATCHER_P(IsListOfSize, n, fmt("Is a list of size [%1%]", n)) {
if (arg.type() != nList) {
*result_listener << "Expected list got " << arg.type();
return false;
} else if (arg.listSize() != (size_t)n) {
*result_listener << "Expected as list of size " << n << " got " << arg.listSize();
return false;
}
return true;
}

MATCHER_P(IsAttrsOfSize, n, fmt("Is a set of size [%1%]", n)) {
if (arg.type() != nAttrs) {
*result_listener << "Expexted set got " << arg.type();
return false;
} else if (arg.attrs->size() != (size_t)n) {
*result_listener << "Expected a set with " << n << " attributes but got " << arg.attrs->size();
return false;
}
return true;
}


} /* namespace nix */
15 changes: 15 additions & 0 deletions src/libexpr/tests/local.mk
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
check: libexpr-tests_RUN

programs += libexpr-tests

libexpr-tests_DIR := $(d)

libexpr-tests_INSTALL_DIR :=

libexpr-tests_SOURCES := $(wildcard $(d)/*.cc)

libexpr-tests_CXXFLAGS += -I src/libexpr -I src/libutil -I src/libstore -I src/libexpr/tests

libexpr-tests_LIBS = libexpr libutil libstore

libexpr-tests_LDFLAGS := $(GTEST_LIBS) -lgmock
Loading

0 comments on commit 059ae7f

Please sign in to comment.