From 8089102164cda23d4beafc3c44aaf1cdecaeb2cf Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 3 Sep 2025 16:08:35 -0400 Subject: [PATCH] Separate internal from non-internal unit tests of the C API This helps us make sure that the external C API is sufficient for the tasks that we think it is sufficient for. --- src/libexpr-tests/meson.build | 1 + src/libexpr-tests/nix_api_expr.cc | 28 +++--- src/libexpr-tests/nix_api_external.cc | 5 +- src/libexpr-tests/nix_api_value.cc | 21 ++--- src/libexpr-tests/nix_api_value_internal.cc | 25 ++++++ src/libstore-tests/nix_api_store.cc | 10 +-- .../include/nix/util/tests/nix_api_util.hh | 8 ++ src/libutil-tests/meson.build | 1 + src/libutil-tests/nix_api_util.cc | 80 +---------------- src/libutil-tests/nix_api_util_internal.cc | 85 +++++++++++++++++++ 10 files changed, 145 insertions(+), 119 deletions(-) create mode 100644 src/libexpr-tests/nix_api_value_internal.cc create mode 100644 src/libutil-tests/nix_api_util_internal.cc diff --git a/src/libexpr-tests/meson.build b/src/libexpr-tests/meson.build index a876e9705..c5dafe0de 100644 --- a/src/libexpr-tests/meson.build +++ b/src/libexpr-tests/meson.build @@ -55,6 +55,7 @@ sources = files( 'nix_api_expr.cc', 'nix_api_external.cc', 'nix_api_value.cc', + 'nix_api_value_internal.cc', 'primops.cc', 'search-path.cc', 'trivial.cc', diff --git a/src/libexpr-tests/nix_api_expr.cc b/src/libexpr-tests/nix_api_expr.cc index 529c2f584..5e0868b6e 100644 --- a/src/libexpr-tests/nix_api_expr.cc +++ b/src/libexpr-tests/nix_api_expr.cc @@ -1,7 +1,5 @@ #include "nix_api_store.h" -#include "nix_api_store_internal.h" #include "nix_api_util.h" -#include "nix_api_util_internal.h" #include "nix_api_expr.h" #include "nix_api_value.h" @@ -151,8 +149,8 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_value) assert_ctx_ok(); auto r = nix_string_realise(ctx, state, value, false); ASSERT_EQ(nullptr, r); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); - ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("cannot coerce"))); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("cannot coerce")); } TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build) @@ -168,8 +166,8 @@ TEST_F(nix_api_expr_test, nix_expr_realise_context_bad_build) assert_ctx_ok(); auto r = nix_string_realise(ctx, state, value, false); ASSERT_EQ(nullptr, r); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); - ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("failed with exit code 1"))); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("failed with exit code 1")); } TEST_F(nix_api_expr_test, nix_expr_realise_context) @@ -381,12 +379,11 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_no_return) nix_value * result = nix_alloc_value(ctx, state); assert_ctx_ok(); nix_value_call(ctx, state, primopValue, three, result); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); ASSERT_THAT( - ctx->last_err, - testing::Optional( - testing::HasSubstr("Implementation error in custom function: return value was not initialized"))); - ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badNoReturn"))); + nix_err_msg(nullptr, ctx, nullptr), + testing::HasSubstr("Implementation error in custom function: return value was not initialized")); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badNoReturn")); } static void primop_bad_return_thunk( @@ -419,12 +416,11 @@ TEST_F(nix_api_expr_test, nix_expr_primop_bad_return_thunk) assert_ctx_ok(); NIX_VALUE_CALL(ctx, state, result, primopValue, toString, four); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); ASSERT_THAT( - ctx->last_err, - testing::Optional( - testing::HasSubstr("Implementation error in custom function: return value must not be a thunk"))); - ASSERT_THAT(ctx->last_err, testing::Optional(testing::HasSubstr("badReturnThunk"))); + nix_err_msg(nullptr, ctx, nullptr), + testing::HasSubstr("Implementation error in custom function: return value must not be a thunk")); + ASSERT_THAT(nix_err_msg(nullptr, ctx, nullptr), testing::HasSubstr("badReturnThunk")); } TEST_F(nix_api_expr_test, nix_value_call_multi_no_args) diff --git a/src/libexpr-tests/nix_api_external.cc b/src/libexpr-tests/nix_api_external.cc index 93da3ca39..ec19f1212 100644 --- a/src/libexpr-tests/nix_api_external.cc +++ b/src/libexpr-tests/nix_api_external.cc @@ -1,9 +1,6 @@ #include "nix_api_store.h" -#include "nix_api_store_internal.h" #include "nix_api_util.h" -#include "nix_api_util_internal.h" #include "nix_api_expr.h" -#include "nix_api_expr_internal.h" #include "nix_api_value.h" #include "nix_api_external.h" @@ -39,7 +36,7 @@ private: std::string type_string = "nix-external_x); type_string += " )>"; - res->str = &*type_string.begin(); + nix_set_string_return(res, &*type_string.begin()); } }; diff --git a/src/libexpr-tests/nix_api_value.cc b/src/libexpr-tests/nix_api_value.cc index 5d85ed68d..af95224de 100644 --- a/src/libexpr-tests/nix_api_value.cc +++ b/src/libexpr-tests/nix_api_value.cc @@ -1,10 +1,7 @@ #include "nix_api_store.h" -#include "nix_api_store_internal.h" #include "nix_api_util.h" -#include "nix_api_util_internal.h" #include "nix_api_expr.h" #include "nix_api_value.h" -#include "nix_api_expr_internal.h" #include "nix/expr/tests/nix_api_expr.hh" #include "nix/util/tests/string_callback.hh" @@ -16,14 +13,6 @@ namespace nixC { -TEST_F(nix_api_expr_test, as_nix_value_ptr) -{ - // nix_alloc_value casts nix::Value to nix_value - // It should be obvious from the decl that that works, but if it doesn't, - // the whole implementation would be utterly broken. - ASSERT_EQ(sizeof(nix::Value), sizeof(nix_value)); -} - TEST_F(nix_api_expr_test, nix_value_get_int_invalid) { ASSERT_EQ(0, nix_get_int(ctx, nullptr)); @@ -320,8 +309,10 @@ TEST_F(nix_api_expr_test, nix_value_init_apply_error) // Evaluate it nix_value_force(ctx, state, v); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); - ASSERT_THAT(ctx->last_err.value(), testing::HasSubstr("attempt to call something which is not a function but")); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); + ASSERT_THAT( + nix_err_msg(nullptr, ctx, nullptr), + testing::HasSubstr("attempt to call something which is not a function but")); // Clean up nix_gc_decref(ctx, some_string); @@ -380,7 +371,9 @@ TEST_F(nix_api_expr_test, nix_value_init_apply_lazy_arg) // nix_get_attr_byname isn't lazy (it could have been) so it will throw the exception nix_value * foo = nix_get_attr_byname(ctx, r, state, "foo"); ASSERT_EQ(nullptr, foo); - ASSERT_THAT(ctx->last_err.value(), testing::HasSubstr("error message for test case nix_value_init_apply_lazy_arg")); + ASSERT_THAT( + nix_err_msg(nullptr, ctx, nullptr), + testing::HasSubstr("error message for test case nix_value_init_apply_lazy_arg")); // Clean up nix_gc_decref(ctx, f); diff --git a/src/libexpr-tests/nix_api_value_internal.cc b/src/libexpr-tests/nix_api_value_internal.cc new file mode 100644 index 000000000..34db6ac81 --- /dev/null +++ b/src/libexpr-tests/nix_api_value_internal.cc @@ -0,0 +1,25 @@ +#include "nix_api_store.h" +#include "nix_api_util.h" +#include "nix_api_expr.h" +#include "nix_api_value.h" +#include "nix_api_expr_internal.h" + +#include "nix/expr/tests/nix_api_expr.hh" +#include "nix/util/tests/string_callback.hh" + +#include +#include +#include +#include + +namespace nixC { + +TEST_F(nix_api_expr_test, as_nix_value_ptr) +{ + // nix_alloc_value casts nix::Value to nix_value + // It should be obvious from the decl that that works, but if it doesn't, + // the whole implementation would be utterly broken. + ASSERT_EQ(sizeof(nix::Value), sizeof(nix_value)); +} + +} // namespace nixC diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index c7146f977..c14fb6d9f 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -1,7 +1,5 @@ #include "nix_api_util.h" -#include "nix_api_util_internal.h" #include "nix_api_store.h" -#include "nix_api_store_internal.h" #include "nix/store/tests/nix_api_store.hh" #include "nix/util/tests/string_callback.hh" @@ -65,7 +63,7 @@ TEST_F(nix_api_store_test, nix_store_get_storedir) TEST_F(nix_api_store_test, InvalidPathFails) { nix_store_parse_path(ctx, store, "invalid-path"); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); } TEST_F(nix_api_store_test, ReturnsValidStorePath) @@ -80,7 +78,7 @@ TEST_F(nix_api_store_test, ReturnsValidStorePath) TEST_F(nix_api_store_test, SetsLastErrCodeToNixOk) { StorePath * path = nix_store_parse_path(ctx, store, (nixStoreDir + PATH_SUFFIX).c_str()); - ASSERT_EQ(ctx->last_err_code, NIX_OK); + ASSERT_EQ(nix_err_code(ctx), NIX_OK); nix_store_path_free(path); } @@ -103,7 +101,7 @@ TEST_F(nix_api_util_context, nix_store_open_dummy) { nix_libstore_init(ctx); Store * store = nix_store_open(ctx, "dummy://", nullptr); - ASSERT_EQ(NIX_OK, ctx->last_err_code); + ASSERT_EQ(NIX_OK, nix_err_code(ctx)); ASSERT_STREQ("dummy://", store->ptr->config.getReference().render(/*withParams=*/true).c_str()); std::string str; @@ -117,7 +115,7 @@ TEST_F(nix_api_util_context, nix_store_open_invalid) { nix_libstore_init(ctx); Store * store = nix_store_open(ctx, "invalid://", nullptr); - ASSERT_EQ(NIX_ERR_NIX_ERROR, ctx->last_err_code); + ASSERT_EQ(NIX_ERR_NIX_ERROR, nix_err_code(ctx)); ASSERT_EQ(nullptr, store); nix_store_free(store); } diff --git a/src/libutil-test-support/include/nix/util/tests/nix_api_util.hh b/src/libutil-test-support/include/nix/util/tests/nix_api_util.hh index 57f7f1ecf..cc1d244f5 100644 --- a/src/libutil-test-support/include/nix/util/tests/nix_api_util.hh +++ b/src/libutil-test-support/include/nix/util/tests/nix_api_util.hh @@ -54,4 +54,12 @@ protected: #define assert_ctx_err() assert_ctx_err(__FILE__, __LINE__) }; +static inline auto createOwnedNixContext() +{ + return std::unique_ptr(nix_c_context_create(), {}); +} + } // namespace nixC diff --git a/src/libutil-tests/meson.build b/src/libutil-tests/meson.build index 0e2a2e468..ff71d2215 100644 --- a/src/libutil-tests/meson.build +++ b/src/libutil-tests/meson.build @@ -63,6 +63,7 @@ sources = files( 'lru-cache.cc', 'monitorfdhup.cc', 'nix_api_util.cc', + 'nix_api_util_internal.cc', 'pool.cc', 'position.cc', 'processes.cc', diff --git a/src/libutil-tests/nix_api_util.cc b/src/libutil-tests/nix_api_util.cc index 9693ab3a5..48f85c403 100644 --- a/src/libutil-tests/nix_api_util.cc +++ b/src/libutil-tests/nix_api_util.cc @@ -1,7 +1,6 @@ #include "nix/util/config-global.hh" #include "nix/util/args.hh" #include "nix_api_util.h" -#include "nix_api_util_internal.h" #include "nix/util/tests/nix_api_util.hh" #include "nix/util/tests/string_callback.hh" @@ -13,41 +12,6 @@ namespace nixC { -TEST_F(nix_api_util_context, nix_context_error) -{ - std::string err_msg_ref; - try { - throw nix::Error("testing error"); - } catch (nix::Error & e) { - err_msg_ref = e.what(); - nix_context_error(ctx); - } - ASSERT_EQ(ctx->last_err_code, NIX_ERR_NIX_ERROR); - ASSERT_EQ(ctx->name, "nix::Error"); - ASSERT_EQ(*ctx->last_err, err_msg_ref); - ASSERT_EQ(ctx->info->msg.str(), "testing error"); - - try { - throw std::runtime_error("testing exception"); - } catch (std::exception & e) { - err_msg_ref = e.what(); - nix_context_error(ctx); - } - ASSERT_EQ(ctx->last_err_code, NIX_ERR_UNKNOWN); - ASSERT_EQ(*ctx->last_err, err_msg_ref); - - nix_clear_err(ctx); - ASSERT_EQ(ctx->last_err_code, NIX_OK); -} - -TEST_F(nix_api_util_context, nix_set_err_msg) -{ - ASSERT_EQ(ctx->last_err_code, NIX_OK); - nix_set_err_msg(ctx, NIX_ERR_UNKNOWN, "unknown test error"); - ASSERT_EQ(ctx->last_err_code, NIX_ERR_UNKNOWN); - ASSERT_EQ(*ctx->last_err, "unknown test error"); -} - TEST(nix_api_util, nix_version_get) { ASSERT_EQ(std::string(nix_version_get()), PACKAGE_VERSION); @@ -61,17 +25,9 @@ struct MySettings : nix::Config MySettings mySettings; static nix::GlobalConfig::Register rs(&mySettings); -static auto createOwnedNixContext() -{ - return std::unique_ptr(nix_c_context_create(), {}); -} - TEST_F(nix_api_util_context, nix_setting_get) { - ASSERT_EQ(ctx->last_err_code, NIX_OK); + ASSERT_EQ(nix_err_code(ctx), NIX_OK); std::string setting_value; nix_err result = nix_setting_get(ctx, "invalid-key", OBSERVE_STRING(setting_value)); ASSERT_EQ(result, NIX_ERR_KEY); @@ -114,40 +70,6 @@ TEST_F(nix_api_util_context, nix_err_msg) ASSERT_EQ(sz, err_msg.size()); } -TEST_F(nix_api_util_context, nix_err_info_msg) -{ - std::string err_info; - - // no error - EXPECT_THROW(nix_err_info_msg(NULL, ctx, OBSERVE_STRING(err_info)), nix::Error); - - try { - throw nix::Error("testing error"); - } catch (...) { - nix_context_error(ctx); - } - auto new_ctx = createOwnedNixContext(); - nix_err_info_msg(new_ctx.get(), ctx, OBSERVE_STRING(err_info)); - ASSERT_STREQ("testing error", err_info.c_str()); -} - -TEST_F(nix_api_util_context, nix_err_name) -{ - std::string err_name; - - // no error - EXPECT_THROW(nix_err_name(NULL, ctx, OBSERVE_STRING(err_name)), nix::Error); - - try { - throw nix::Error("testing error"); - } catch (...) { - nix_context_error(ctx); - } - auto new_ctx = createOwnedNixContext(); - nix_err_name(new_ctx.get(), ctx, OBSERVE_STRING(err_name)); - ASSERT_EQ(std::string(err_name), "nix::Error"); -} - TEST_F(nix_api_util_context, nix_err_code) { ASSERT_EQ(nix_err_code(ctx), NIX_OK); diff --git a/src/libutil-tests/nix_api_util_internal.cc b/src/libutil-tests/nix_api_util_internal.cc new file mode 100644 index 000000000..6fb0a623f --- /dev/null +++ b/src/libutil-tests/nix_api_util_internal.cc @@ -0,0 +1,85 @@ +#include "nix/util/config-global.hh" +#include "nix/util/args.hh" +#include "nix_api_util.h" +#include "nix_api_util_internal.h" +#include "nix/util/tests/nix_api_util.hh" +#include "nix/util/tests/string_callback.hh" + +#include + +#include + +#include "util-tests-config.hh" + +namespace nixC { + +TEST_F(nix_api_util_context, nix_context_error) +{ + std::string err_msg_ref; + try { + throw nix::Error("testing error"); + } catch (nix::Error & e) { + err_msg_ref = e.what(); + nix_context_error(ctx); + } + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_NIX_ERROR); + ASSERT_EQ(ctx->name, "nix::Error"); + ASSERT_EQ(*ctx->last_err, err_msg_ref); + ASSERT_EQ(ctx->info->msg.str(), "testing error"); + + try { + throw std::runtime_error("testing exception"); + } catch (std::exception & e) { + err_msg_ref = e.what(); + nix_context_error(ctx); + } + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_UNKNOWN); + ASSERT_EQ(*ctx->last_err, err_msg_ref); + + nix_clear_err(ctx); + ASSERT_EQ(nix_err_code(ctx), NIX_OK); +} + +TEST_F(nix_api_util_context, nix_set_err_msg) +{ + ASSERT_EQ(nix_err_code(ctx), NIX_OK); + nix_set_err_msg(ctx, NIX_ERR_UNKNOWN, "unknown test error"); + ASSERT_EQ(nix_err_code(ctx), NIX_ERR_UNKNOWN); + ASSERT_EQ(*ctx->last_err, "unknown test error"); +} + +TEST_F(nix_api_util_context, nix_err_info_msg) +{ + std::string err_info; + + // no error + EXPECT_THROW(nix_err_info_msg(NULL, ctx, OBSERVE_STRING(err_info)), nix::Error); + + try { + throw nix::Error("testing error"); + } catch (...) { + nix_context_error(ctx); + } + auto new_ctx = createOwnedNixContext(); + nix_err_info_msg(new_ctx.get(), ctx, OBSERVE_STRING(err_info)); + ASSERT_STREQ("testing error", err_info.c_str()); +} + +TEST_F(nix_api_util_context, nix_err_name) +{ + std::string err_name; + + // no error + EXPECT_THROW(nix_err_name(NULL, ctx, OBSERVE_STRING(err_name)), nix::Error); + + try { + throw nix::Error("testing error"); + } catch (...) { + nix_context_error(ctx); + } + auto new_ctx = createOwnedNixContext(); + nix_err_name(new_ctx.get(), ctx, OBSERVE_STRING(err_name)); + ASSERT_EQ(std::string(err_name), "nix::Error"); +} + +} // namespace nixC