Skip to content

Commit 54c0512

Browse files
seirlcopybara-github
seirl
authored and
copybara-github
committed
Support cel::ActivationInterface in CelTestContext.
Update `CelTestContext` and `CelActivationFactoryFn` to return `std::unique_ptr<cel::ActivationInterface>` instead of `cel::Activation` by value. Previously, `CelTestContext::CelActivationFactoryFn` was hardcoded to return `absl::StatusOr<cel::Activation>`. 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<cel::ActivationInterface>` from the factory function. PiperOrigin-RevId: 956045618
1 parent d96768d commit 54c0512

4 files changed

Lines changed: 143 additions & 25 deletions

File tree

‎testing/testrunner/BUILD‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ cc_library(
1818
"//eval/public:cel_expression",
1919
"//runtime",
2020
"//runtime:activation",
21+
"//runtime:activation_interface",
2122
"@com_google_absl//absl/base:nullability",
2223
"@com_google_absl//absl/container:flat_hash_map",
2324
"@com_google_absl//absl/memory",
@@ -51,6 +52,7 @@ cc_library(
5152
"//internal:testing_no_main",
5253
"//runtime",
5354
"//runtime:activation",
55+
"//runtime:activation_interface",
5456
"@com_google_absl//absl/functional:overload",
5557
"@com_google_absl//absl/status",
5658
"@com_google_absl//absl/status:status_matchers",
@@ -90,6 +92,7 @@ cc_test(
9092
":cel_test_context",
9193
":coverage_index",
9294
":runner_lib",
95+
"//base:attributes",
9396
"//checker:type_checker_builder",
9497
"//checker:validation_result",
9598
"//common:ast_proto",
@@ -107,6 +110,8 @@ cc_test(
107110
"//internal:testing_descriptor_pool",
108111
"//runtime",
109112
"//runtime:activation",
113+
"//runtime:activation_interface",
114+
"//runtime:function_overload_reference",
110115
"//runtime:runtime_builder",
111116
"//runtime:standard_runtime_builder_factory",
112117
"@com_google_absl//absl/container:flat_hash_map",
@@ -115,6 +120,7 @@ cc_test(
115120
"@com_google_absl//absl/status:status_matchers",
116121
"@com_google_absl//absl/status:statusor",
117122
"@com_google_absl//absl/strings:string_view",
123+
"@com_google_absl//absl/types:span",
118124
"@com_google_cel_spec//proto/cel/expr/conformance/proto3:test_all_types_cc_proto",
119125
"@com_google_cel_spec//proto/cel/expr/conformance/test:suite_cc_proto",
120126
"@com_google_protobuf//:protobuf",

‎testing/testrunner/cel_test_context.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include "common/value.h"
3030
#include "compiler/compiler.h"
3131
#include "eval/public/cel_expression.h"
32-
#include "runtime/activation.h"
32+
#include "runtime/activation_interface.h"
3333
#include "runtime/runtime.h"
3434
#include "testing/testrunner/cel_expression_source.h"
3535
#include "cel/expr/conformance/test/suite.pb.h"
@@ -40,9 +40,10 @@ namespace cel::test {
4040
// compiled CEL expressions.
4141
class CelTestContext {
4242
public:
43-
using CelActivationFactoryFn = std::function<absl::StatusOr<cel::Activation>(
44-
const cel::expr::conformance::test::TestCase& test_case,
45-
google::protobuf::Arena* arena)>;
43+
using CelActivationFactoryFn =
44+
std::function<absl::StatusOr<std::unique_ptr<cel::ActivationInterface>>(
45+
const cel::expr::conformance::test::TestCase& test_case,
46+
google::protobuf::Arena* arena)>;
4647
using AssertFn = std::function<void(
4748
const cel::Value& computed,
4849
const cel::expr::conformance::test::TestCase& test_case,

‎testing/testrunner/runner_lib.cc‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
#include "cel/expr/eval.pb.h"
2424
#include "absl/functional/overload.h"
2525
#include "absl/status/status.h"
26-
#include "absl/status/status_matchers.h"
2726
#include "absl/status/statusor.h"
2827
#include "absl/strings/str_cat.h"
2928
#include "absl/strings/string_view.h"
@@ -39,6 +38,7 @@
3938
#include "internal/status_macros.h"
4039
#include "internal/testing.h"
4140
#include "runtime/activation.h"
41+
#include "runtime/activation_interface.h"
4242
#include "runtime/runtime.h"
4343
#include "testing/testrunner/cel_expression_source.h"
4444
#include "testing/testrunner/cel_test_context.h"
@@ -120,7 +120,7 @@ google::protobuf::MessageFactory* GetMessageFactory(const CelTestContext& contex
120120

121121
absl::StatusOr<cel::Value> EvalWithModernBindings(
122122
const CheckedExpr& checked_expr, const CelTestContext& context,
123-
const cel::Activation& activation, google::protobuf::Arena* arena) {
123+
const cel::ActivationInterface& activation, google::protobuf::Arena* arena) {
124124
CEL_ASSIGN_OR_RETURN(std::unique_ptr<cel::Program> program,
125125
Plan(checked_expr, context.runtime()));
126126
return program->Evaluate(arena, activation);
@@ -205,25 +205,37 @@ absl::Status AddTestCaseBindingsToModernActivation(
205205
return absl::OkStatus();
206206
}
207207

208-
absl::StatusOr<cel::Activation> GetActivation(const CelTestContext& context,
209-
const TestCase& test_case,
210-
google::protobuf::Arena* arena) {
208+
absl::StatusOr<std::unique_ptr<cel::ActivationInterface>> GetActivation(
209+
const CelTestContext& context, const TestCase& test_case,
210+
google::protobuf::Arena* arena) {
211211
if (context.activation_factory() != nullptr) {
212212
return context.activation_factory()(test_case, arena);
213213
}
214-
return cel::Activation();
214+
return std::make_unique<cel::Activation>();
215215
}
216216

217-
absl::StatusOr<cel::Activation> CreateModernActivationFromBindings(
218-
const TestCase& test_case, const CelTestContext& context,
219-
google::protobuf::Arena* arena) {
220-
CEL_ASSIGN_OR_RETURN(cel::Activation activation,
217+
absl::StatusOr<std::unique_ptr<cel::ActivationInterface>>
218+
CreateModernActivationFromBindings(const TestCase& test_case,
219+
const CelTestContext& context,
220+
google::protobuf::Arena* arena) {
221+
CEL_ASSIGN_OR_RETURN(std::unique_ptr<cel::ActivationInterface> activation,
221222
GetActivation(context, test_case, arena));
222-
CEL_RETURN_IF_ERROR(
223-
AddCustomBindingsToModernActivation(context, activation, arena));
224223

225-
CEL_RETURN_IF_ERROR(AddTestCaseBindingsToModernActivation(test_case, context,
226-
activation, arena));
224+
const bool has_custom_bindings =
225+
!context.custom_bindings().empty() || !test_case.input().empty();
226+
if (has_custom_bindings) {
227+
auto* cel_activation = dynamic_cast<cel::Activation*>(activation.get());
228+
if (cel_activation == nullptr) {
229+
return absl::InvalidArgumentError(
230+
"Custom bindings or test case input bindings cannot be combined with "
231+
"a custom cel::ActivationInterface implementation returned by "
232+
"activation_factory.");
233+
}
234+
CEL_RETURN_IF_ERROR(
235+
AddCustomBindingsToModernActivation(context, *cel_activation, arena));
236+
CEL_RETURN_IF_ERROR(AddTestCaseBindingsToModernActivation(
237+
test_case, context, *cel_activation, arena));
238+
}
227239

228240
return activation;
229241
}
@@ -362,9 +374,9 @@ absl::StatusOr<cel::Value> TestRunner::EvalWithRuntime(
362374
const CheckedExpr& checked_expr, const TestCase& test_case,
363375
google::protobuf::Arena* arena) {
364376
CEL_ASSIGN_OR_RETURN(
365-
cel::Activation activation,
377+
std::unique_ptr<cel::ActivationInterface> activation,
366378
CreateModernActivationFromBindings(test_case, *test_context_, arena));
367-
return EvalWithModernBindings(checked_expr, *test_context_, activation,
379+
return EvalWithModernBindings(checked_expr, *test_context_, *activation,
368380
arena);
369381
}
370382

‎testing/testrunner/runner_lib_test.cc‎

Lines changed: 104 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <memory>
1818
#include <string>
1919
#include <utility>
20+
#include <vector>
2021

2122
#include "gtest/gtest-spi.h"
2223
#include "absl/container/flat_hash_map.h"
@@ -25,6 +26,8 @@
2526
#include "absl/status/status_matchers.h"
2627
#include "absl/status/statusor.h"
2728
#include "absl/strings/string_view.h"
29+
#include "absl/types/span.h"
30+
#include "base/attribute.h"
2831
#include "checker/type_checker_builder.h"
2932
#include "checker/validation_result.h"
3033
#include "common/ast_proto.h"
@@ -41,6 +44,8 @@
4144
#include "internal/testing.h"
4245
#include "internal/testing_descriptor_pool.h"
4346
#include "runtime/activation.h"
47+
#include "runtime/activation_interface.h"
48+
#include "runtime/function_overload_reference.h"
4449
#include "runtime/runtime.h"
4550
#include "runtime/runtime_builder.h"
4651
#include "runtime/standard_runtime_builder_factory.h"
@@ -625,11 +630,11 @@ TEST(TestRunnerStandaloneTest, BasicTestWithActivationFactorySucceeds) {
625630
std::unique_ptr<CelTestContext> context =
626631
CelTestContext::CreateFromRuntime(std::move(runtime));
627632
context->SetActivationFactory(
628-
[](const TestCase& test_case,
629-
google::protobuf::Arena* arena) -> absl::StatusOr<cel::Activation> {
630-
cel::Activation activation;
631-
activation.InsertOrAssignValue("x", cel::IntValue(10));
632-
activation.InsertOrAssignValue("y", cel::IntValue(5));
633+
[](const TestCase& test_case, google::protobuf::Arena* arena)
634+
-> absl::StatusOr<std::unique_ptr<cel::ActivationInterface>> {
635+
auto activation = std::make_unique<cel::Activation>();
636+
activation->InsertOrAssignValue("x", cel::IntValue(10));
637+
activation->InsertOrAssignValue("y", cel::IntValue(5));
633638
return activation;
634639
});
635640
context->SetExpressionSource(
@@ -652,6 +657,100 @@ TEST(TestRunnerStandaloneTest, BasicTestWithActivationFactorySucceeds) {
652657
EXPECT_NO_FATAL_FAILURE(test_runner.RunTest(test_case));
653658
}
654659

660+
namespace {
661+
class TestCustomActivation : public cel::ActivationInterface {
662+
public:
663+
absl::StatusOr<bool> FindVariable(
664+
absl::string_view name, const google::protobuf::DescriptorPool* descriptor_pool,
665+
google::protobuf::MessageFactory* message_factory, google::protobuf::Arena* arena,
666+
cel::Value* result) const override {
667+
if (name == "x") {
668+
*result = cel::IntValue(100);
669+
return true;
670+
}
671+
if (name == "y") {
672+
*result = cel::IntValue(200);
673+
return true;
674+
}
675+
return false;
676+
}
677+
678+
std::vector<cel::FunctionOverloadReference> FindFunctionOverloads(
679+
absl::string_view name) const override {
680+
return {};
681+
}
682+
683+
absl::Span<const cel::AttributePattern> GetUnknownAttributes()
684+
const override {
685+
return {};
686+
}
687+
688+
absl::Span<const cel::AttributePattern> GetMissingAttributes()
689+
const override {
690+
return {};
691+
}
692+
};
693+
} // namespace
694+
695+
TEST(TestRunnerStandaloneTest, CustomActivationInterfaceFactorySucceeds) {
696+
ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result,
697+
DefaultCompiler().Compile("x + y"));
698+
CheckedExpr checked_expr;
699+
ASSERT_THAT(cel::AstToCheckedExpr(*validation_result.GetAst(), &checked_expr),
700+
absl_testing::IsOk());
701+
702+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<const cel::Runtime> runtime,
703+
CreateTestRuntime());
704+
std::unique_ptr<CelTestContext> context =
705+
CelTestContext::CreateFromRuntime(std::move(runtime));
706+
context->SetActivationFactory(
707+
[](const TestCase& test_case, google::protobuf::Arena* arena)
708+
-> absl::StatusOr<std::unique_ptr<cel::ActivationInterface>> {
709+
return std::make_unique<TestCustomActivation>();
710+
});
711+
context->SetExpressionSource(
712+
CelExpressionSource::FromCheckedExpr(std::move(checked_expr)));
713+
714+
TestCase test_case = ParseTextProtoOrDie<TestCase>(R"pb(
715+
output { result_value { int64_value: 300 } }
716+
)pb");
717+
TestRunner test_runner(std::move(context));
718+
EXPECT_NO_FATAL_FAILURE(test_runner.RunTest(test_case));
719+
}
720+
721+
TEST(TestRunnerStandaloneTest,
722+
CustomActivationInterfaceWithInputsReturnsError) {
723+
ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result,
724+
DefaultCompiler().Compile("x + y"));
725+
CheckedExpr checked_expr;
726+
ASSERT_THAT(cel::AstToCheckedExpr(*validation_result.GetAst(), &checked_expr),
727+
absl_testing::IsOk());
728+
729+
ASSERT_OK_AND_ASSIGN(std::unique_ptr<const cel::Runtime> runtime,
730+
CreateTestRuntime());
731+
std::unique_ptr<CelTestContext> context =
732+
CelTestContext::CreateFromRuntime(std::move(runtime));
733+
context->SetActivationFactory(
734+
[](const TestCase& test_case, google::protobuf::Arena* arena)
735+
-> absl::StatusOr<std::unique_ptr<cel::ActivationInterface>> {
736+
return std::make_unique<TestCustomActivation>();
737+
});
738+
context->SetExpressionSource(
739+
CelExpressionSource::FromCheckedExpr(std::move(checked_expr)));
740+
741+
static auto* test_case_ptr = new TestCase(ParseTextProtoOrDie<TestCase>(R"pb(
742+
input {
743+
key: "x"
744+
value { value { int64_value: 4 } }
745+
}
746+
output { result_value { int64_value: 300 } }
747+
)pb"));
748+
static auto* test_runner_ptr = new TestRunner(std::move(context));
749+
EXPECT_FATAL_FAILURE(
750+
test_runner_ptr->RunTest(*test_case_ptr),
751+
"Custom bindings or test case input bindings cannot be combined");
752+
}
753+
655754
TEST(TestRunnerStandaloneTest, CustomAssertFnIsUsed) {
656755
// Compile the expression.
657756
ASSERT_OK_AND_ASSIGN(cel::ValidationResult validation_result,

0 commit comments

Comments
 (0)