From 54c0512418805e20491cae01b40452f4a2864d53 Mon Sep 17 00:00:00 2001 From: Antoine Pietri Date: Wed, 29 Jul 2026 13:06:46 -0700 Subject: [PATCH] Support cel::ActivationInterface in CelTestContext. Update `CelTestContext` and `CelActivationFactoryFn` to return `std::unique_ptr` instead of `cel::Activation` by value. Previously, `CelTestContext::CelActivationFactoryFn` was hardcoded to return `absl::StatusOr`. This overlooked the fact that `cel::Runtime::Evaluate` accepts `const cel::ActivationInterface&` and prevents custom or lazy activations that implement `cel::ActivationInterface` directly from being returned by the factory without object slicing or compilation errors. `CelTestContext` and `TestRunner` now accept `std::unique_ptr` from the factory function. PiperOrigin-RevId: 956045618 --- testing/testrunner/BUILD | 6 ++ testing/testrunner/cel_test_context.h | 9 ++- testing/testrunner/runner_lib.cc | 44 +++++++---- testing/testrunner/runner_lib_test.cc | 109 ++++++++++++++++++++++++-- 4 files changed, 143 insertions(+), 25 deletions(-) diff --git a/testing/testrunner/BUILD b/testing/testrunner/BUILD index b80167487..f01f892f3 100644 --- a/testing/testrunner/BUILD +++ b/testing/testrunner/BUILD @@ -18,6 +18,7 @@ cc_library( "//eval/public:cel_expression", "//runtime", "//runtime:activation", + "//runtime:activation_interface", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/memory", @@ -51,6 +52,7 @@ cc_library( "//internal:testing_no_main", "//runtime", "//runtime:activation", + "//runtime:activation_interface", "@com_google_absl//absl/functional:overload", "@com_google_absl//absl/status", "@com_google_absl//absl/status:status_matchers", @@ -90,6 +92,7 @@ cc_test( ":cel_test_context", ":coverage_index", ":runner_lib", + "//base:attributes", "//checker:type_checker_builder", "//checker:validation_result", "//common:ast_proto", @@ -107,6 +110,8 @@ cc_test( "//internal:testing_descriptor_pool", "//runtime", "//runtime:activation", + "//runtime:activation_interface", + "//runtime:function_overload_reference", "//runtime:runtime_builder", "//runtime:standard_runtime_builder_factory", "@com_google_absl//absl/container:flat_hash_map", @@ -115,6 +120,7 @@ cc_test( "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", "@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto", "@com_google_cel_spec//proto/cel/expr/conformance/test:suite_cc_proto", "@com_google_protobuf//:protobuf", diff --git a/testing/testrunner/cel_test_context.h b/testing/testrunner/cel_test_context.h index 0e0f21e28..988c37c66 100644 --- a/testing/testrunner/cel_test_context.h +++ b/testing/testrunner/cel_test_context.h @@ -29,7 +29,7 @@ #include "common/value.h" #include "compiler/compiler.h" #include "eval/public/cel_expression.h" -#include "runtime/activation.h" +#include "runtime/activation_interface.h" #include "runtime/runtime.h" #include "testing/testrunner/cel_expression_source.h" #include "cel/expr/conformance/test/suite.pb.h" @@ -40,9 +40,10 @@ namespace cel::test { // compiled CEL expressions. class CelTestContext { public: - using CelActivationFactoryFn = std::function( - const cel::expr::conformance::test::TestCase& test_case, - google::protobuf::Arena* arena)>; + using CelActivationFactoryFn = + std::function>( + const cel::expr::conformance::test::TestCase& test_case, + google::protobuf::Arena* arena)>; using AssertFn = std::function EvalWithModernBindings( const CheckedExpr& checked_expr, const CelTestContext& context, - const cel::Activation& activation, google::protobuf::Arena* arena) { + const cel::ActivationInterface& activation, google::protobuf::Arena* arena) { CEL_ASSIGN_OR_RETURN(std::unique_ptr program, Plan(checked_expr, context.runtime())); return program->Evaluate(arena, activation); @@ -205,25 +205,37 @@ absl::Status AddTestCaseBindingsToModernActivation( return absl::OkStatus(); } -absl::StatusOr GetActivation(const CelTestContext& context, - const TestCase& test_case, - google::protobuf::Arena* arena) { +absl::StatusOr> GetActivation( + const CelTestContext& context, const TestCase& test_case, + google::protobuf::Arena* arena) { if (context.activation_factory() != nullptr) { return context.activation_factory()(test_case, arena); } - return cel::Activation(); + return std::make_unique(); } -absl::StatusOr CreateModernActivationFromBindings( - const TestCase& test_case, const CelTestContext& context, - google::protobuf::Arena* arena) { - CEL_ASSIGN_OR_RETURN(cel::Activation activation, +absl::StatusOr> +CreateModernActivationFromBindings(const TestCase& test_case, + const CelTestContext& context, + google::protobuf::Arena* arena) { + CEL_ASSIGN_OR_RETURN(std::unique_ptr activation, GetActivation(context, test_case, arena)); - CEL_RETURN_IF_ERROR( - AddCustomBindingsToModernActivation(context, activation, arena)); - CEL_RETURN_IF_ERROR(AddTestCaseBindingsToModernActivation(test_case, context, - activation, arena)); + const bool has_custom_bindings = + !context.custom_bindings().empty() || !test_case.input().empty(); + if (has_custom_bindings) { + auto* cel_activation = dynamic_cast(activation.get()); + if (cel_activation == nullptr) { + return absl::InvalidArgumentError( + "Custom bindings or test case input bindings cannot be combined with " + "a custom cel::ActivationInterface implementation returned by " + "activation_factory."); + } + CEL_RETURN_IF_ERROR( + AddCustomBindingsToModernActivation(context, *cel_activation, arena)); + CEL_RETURN_IF_ERROR(AddTestCaseBindingsToModernActivation( + test_case, context, *cel_activation, arena)); + } return activation; } @@ -362,9 +374,9 @@ absl::StatusOr TestRunner::EvalWithRuntime( const CheckedExpr& checked_expr, const TestCase& test_case, google::protobuf::Arena* arena) { CEL_ASSIGN_OR_RETURN( - cel::Activation activation, + std::unique_ptr activation, CreateModernActivationFromBindings(test_case, *test_context_, arena)); - return EvalWithModernBindings(checked_expr, *test_context_, activation, + return EvalWithModernBindings(checked_expr, *test_context_, *activation, arena); } diff --git a/testing/testrunner/runner_lib_test.cc b/testing/testrunner/runner_lib_test.cc index 804826b6c..3b0e3f586 100644 --- a/testing/testrunner/runner_lib_test.cc +++ b/testing/testrunner/runner_lib_test.cc @@ -17,6 +17,7 @@ #include #include #include +#include #include "gtest/gtest-spi.h" #include "absl/container/flat_hash_map.h" @@ -25,6 +26,8 @@ #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" +#include "base/attribute.h" #include "checker/type_checker_builder.h" #include "checker/validation_result.h" #include "common/ast_proto.h" @@ -41,6 +44,8 @@ #include "internal/testing.h" #include "internal/testing_descriptor_pool.h" #include "runtime/activation.h" +#include "runtime/activation_interface.h" +#include "runtime/function_overload_reference.h" #include "runtime/runtime.h" #include "runtime/runtime_builder.h" #include "runtime/standard_runtime_builder_factory.h" @@ -625,11 +630,11 @@ TEST(TestRunnerStandaloneTest, BasicTestWithActivationFactorySucceeds) { std::unique_ptr context = CelTestContext::CreateFromRuntime(std::move(runtime)); context->SetActivationFactory( - [](const TestCase& test_case, - google::protobuf::Arena* arena) -> absl::StatusOr { - cel::Activation activation; - activation.InsertOrAssignValue("x", cel::IntValue(10)); - activation.InsertOrAssignValue("y", cel::IntValue(5)); + [](const TestCase& test_case, google::protobuf::Arena* arena) + -> absl::StatusOr> { + auto activation = std::make_unique(); + activation->InsertOrAssignValue("x", cel::IntValue(10)); + activation->InsertOrAssignValue("y", cel::IntValue(5)); return activation; }); context->SetExpressionSource( @@ -652,6 +657,100 @@ TEST(TestRunnerStandaloneTest, BasicTestWithActivationFactorySucceeds) { EXPECT_NO_FATAL_FAILURE(test_runner.RunTest(test_case)); } +namespace { +class TestCustomActivation : public cel::ActivationInterface { + public: + absl::StatusOr FindVariable( + absl::string_view name, const google::protobuf::DescriptorPool* descriptor_pool, + google::protobuf::MessageFactory* message_factory, google::protobuf::Arena* arena, + cel::Value* result) const override { + if (name == "x") { + *result = cel::IntValue(100); + return true; + } + if (name == "y") { + *result = cel::IntValue(200); + return true; + } + return false; + } + + std::vector FindFunctionOverloads( + absl::string_view name) const override { + return {}; + } + + absl::Span GetUnknownAttributes() + const override { + return {}; + } + + absl::Span GetMissingAttributes() + const override { + return {}; + } +}; +} // namespace + +TEST(TestRunnerStandaloneTest, CustomActivationInterfaceFactorySucceeds) { + ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result, + DefaultCompiler().Compile("x + y")); + CheckedExpr checked_expr; + ASSERT_THAT(cel::AstToCheckedExpr(*validation_result.GetAst(), &checked_expr), + absl_testing::IsOk()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr runtime, + CreateTestRuntime()); + std::unique_ptr context = + CelTestContext::CreateFromRuntime(std::move(runtime)); + context->SetActivationFactory( + [](const TestCase& test_case, google::protobuf::Arena* arena) + -> absl::StatusOr> { + return std::make_unique(); + }); + context->SetExpressionSource( + CelExpressionSource::FromCheckedExpr(std::move(checked_expr))); + + TestCase test_case = ParseTextProtoOrDie(R"pb( + output { result_value { int64_value: 300 } } + )pb"); + TestRunner test_runner(std::move(context)); + EXPECT_NO_FATAL_FAILURE(test_runner.RunTest(test_case)); +} + +TEST(TestRunnerStandaloneTest, + CustomActivationInterfaceWithInputsReturnsError) { + ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result, + DefaultCompiler().Compile("x + y")); + CheckedExpr checked_expr; + ASSERT_THAT(cel::AstToCheckedExpr(*validation_result.GetAst(), &checked_expr), + absl_testing::IsOk()); + + ASSERT_OK_AND_ASSIGN(std::unique_ptr runtime, + CreateTestRuntime()); + std::unique_ptr context = + CelTestContext::CreateFromRuntime(std::move(runtime)); + context->SetActivationFactory( + [](const TestCase& test_case, google::protobuf::Arena* arena) + -> absl::StatusOr> { + return std::make_unique(); + }); + context->SetExpressionSource( + CelExpressionSource::FromCheckedExpr(std::move(checked_expr))); + + static auto* test_case_ptr = new TestCase(ParseTextProtoOrDie(R"pb( + input { + key: "x" + value { value { int64_value: 4 } } + } + output { result_value { int64_value: 300 } } + )pb")); + static auto* test_runner_ptr = new TestRunner(std::move(context)); + EXPECT_FATAL_FAILURE( + test_runner_ptr->RunTest(*test_case_ptr), + "Custom bindings or test case input bindings cannot be combined"); +} + TEST(TestRunnerStandaloneTest, CustomAssertFnIsUsed) { // Compile the expression. ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result,