Skip to content

Commit df735c5

Browse files
jckingcopybara-github
jcking
authored and
copybara-github
committed
Change Activation attribute pattern mutators to return absl::Status
PiperOrigin-RevId: 995241250
1 parent 68f523b commit df735c5

21 files changed

Lines changed: 296 additions & 192 deletions

‎eval/compiler/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,6 +511,7 @@ cc_test(
511511
"//runtime/internal:runtime_env_testing",
512512
"@com_google_absl//absl/log:absl_check",
513513
"@com_google_absl//absl/status",
514+
"@com_google_absl//absl/status:status_matchers",
514515
"@com_google_absl//absl/strings",
515516
"@com_google_protobuf//:protobuf",
516517
],

‎eval/compiler/flat_expr_builder_comprehensions_test.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ namespace google::api::expr::runtime {
4343

4444
namespace {
4545

46+
using ::absl_testing::IsOk;
4647
using ::absl_testing::StatusIs;
4748
using ::cel::runtime_internal::NewTestingRuntimeEnv;
4849
using ::cel::expr::CheckedExpr;
@@ -152,9 +153,11 @@ TEST_P(CelExpressionBuilderFlatImplComprehensionsTest, ListCompWithUnknowns) {
152153
&parsed_expr.source_info()));
153154

154155
Activation activation;
155-
activation.set_unknown_attribute_patterns({CelAttributePattern{
156-
"items",
157-
{CreateCelAttributeQualifierPattern(CelValue::CreateInt64(1))}}});
156+
ASSERT_THAT(
157+
activation.SetUnknownAttributePatterns({CelAttributePattern{
158+
"items",
159+
{CreateCelAttributeQualifierPattern(CelValue::CreateInt64(1))}}}),
160+
IsOk());
158161
ContainerBackedListImpl list_impl = ContainerBackedListImpl({
159162
CelValue::CreateInt64(1),
160163
// element items[1] is marked unknown, so the computation should produce

‎eval/compiler/flat_expr_builder_short_circuiting_conformance_test.cc‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include "absl/log/absl_check.h"
88
#include "absl/status/status.h"
9+
#include "absl/status/status_matchers.h"
910
#include "absl/strings/str_cat.h"
1011
#include "absl/strings/string_view.h"
1112
#include "eval/compiler/cel_expression_builder_flat_impl.h"
@@ -26,6 +27,7 @@ namespace google::api::expr::runtime {
2627

2728
namespace {
2829

30+
using ::absl_testing::IsOk;
2931
using ::cel::runtime_internal::NewTestingRuntimeEnv;
3032
using ::cel::expr::Expr;
3133
using ::google::api::expr::parser::Parse;
@@ -187,7 +189,9 @@ TEST_P(ShortCircuitingTest, UnknownAnd) {
187189
auto builder = GetBuilder(/* enable_unknowns=*/true);
188190
absl::Status error = absl::InternalError("error");
189191

190-
activation.set_unknown_attribute_patterns({CelAttributePattern("var1", {})});
192+
ASSERT_THAT(
193+
activation.SetUnknownAttributePatterns({CelAttributePattern("var1", {})}),
194+
IsOk());
191195
activation.InsertValue("var2", CelValue::CreateError(&error));
192196
activation.InsertValue("var3", CelValue::CreateBool(false));
193197

@@ -217,7 +221,9 @@ TEST_P(ShortCircuitingTest, UnknownOr) {
217221
auto builder = GetBuilder(/* enable_unknowns=*/true);
218222
absl::Status error = absl::InternalError("error");
219223

220-
activation.set_unknown_attribute_patterns({CelAttributePattern("var1", {})});
224+
ASSERT_THAT(
225+
activation.SetUnknownAttributePatterns({CelAttributePattern("var1", {})}),
226+
IsOk());
221227
activation.InsertValue("var2", CelValue::CreateError(&error));
222228
activation.InsertValue("var3", CelValue::CreateBool(true));
223229

@@ -307,7 +313,9 @@ TEST_P(ShortCircuitingTest, TernaryUnknownCondHandling) {
307313
activation.InsertValue("arg1", CelValue::CreateError(&error));
308314
activation.InsertValue("arg2", CelValue::CreateInt64(-1));
309315

310-
activation.set_unknown_attribute_patterns({CelAttributePattern("cond", {})});
316+
ASSERT_THAT(
317+
activation.SetUnknownAttributePatterns({CelAttributePattern("cond", {})}),
318+
IsOk());
311319

312320
CelValue result;
313321
ASSERT_NO_FATAL_FAILURE(
@@ -319,9 +327,11 @@ TEST_P(ShortCircuitingTest, TernaryUnknownCondHandling) {
319327
EXPECT_THAT(attrs.begin()->variable_name(), Eq("cond"));
320328

321329
// Unknown branches are discarded if condition is unknown
322-
activation.set_unknown_attribute_patterns({CelAttributePattern("cond", {}),
323-
CelAttributePattern("arg1", {}),
324-
CelAttributePattern("arg2", {})});
330+
ASSERT_THAT(
331+
activation.SetUnknownAttributePatterns({CelAttributePattern("cond", {}),
332+
CelAttributePattern("arg1", {}),
333+
CelAttributePattern("arg2", {})}),
334+
IsOk());
325335

326336
ASSERT_NO_FATAL_FAILURE(
327337
BuildAndEval(builder.get(), expr, activation, &arena, &result));
@@ -344,7 +354,9 @@ TEST_P(ShortCircuitingTest, TernaryUnknownArgsHandling) {
344354
activation.InsertValue("arg2", CelValue::CreateInt64(-1));
345355

346356
// Unknown arg is discarded if condition chooses other branch.
347-
activation.set_unknown_attribute_patterns({CelAttributePattern("arg1", {})});
357+
ASSERT_THAT(
358+
activation.SetUnknownAttributePatterns({CelAttributePattern("arg1", {})}),
359+
IsOk());
348360

349361
CelValue result;
350362

@@ -354,8 +366,10 @@ TEST_P(ShortCircuitingTest, TernaryUnknownArgsHandling) {
354366
EXPECT_EQ(result.Int64OrDie(), -1);
355367

356368
// Branches won't merge if both are unknown.
357-
activation.set_unknown_attribute_patterns(
358-
{CelAttributePattern("arg1", {}), CelAttributePattern("arg2", {})});
369+
ASSERT_THAT(
370+
activation.SetUnknownAttributePatterns(
371+
{CelAttributePattern("arg1", {}), CelAttributePattern("arg2", {})}),
372+
IsOk());
359373

360374
ASSERT_NO_FATAL_FAILURE(
361375
BuildAndEval(builder.get(), expr, activation, &arena, &result));
@@ -378,8 +392,10 @@ TEST_P(ShortCircuitingTest, TernaryUnknownAndErrorHandling) {
378392
activation.InsertValue("arg2", CelValue::CreateInt64(-1));
379393

380394
// Error cond discards args
381-
activation.set_unknown_attribute_patterns(
382-
{CelAttributePattern("arg1", {}), CelAttributePattern("arg2", {})});
395+
ASSERT_THAT(
396+
activation.SetUnknownAttributePatterns(
397+
{CelAttributePattern("arg1", {}), CelAttributePattern("arg2", {})}),
398+
IsOk());
383399

384400
CelValue result;
385401

@@ -389,7 +405,9 @@ TEST_P(ShortCircuitingTest, TernaryUnknownAndErrorHandling) {
389405
EXPECT_EQ(*result.ErrorOrDie(), error);
390406

391407
// Error arg discarded if condition unknown
392-
activation.set_unknown_attribute_patterns({CelAttributePattern("cond", {})});
408+
ASSERT_THAT(
409+
activation.SetUnknownAttributePatterns({CelAttributePattern("cond", {})}),
410+
IsOk());
393411
ASSERT_TRUE(activation.RemoveValueEntry("arg1"));
394412
activation.InsertValue("arg1", CelValue::CreateError(&error));
395413

‎eval/compiler/flat_expr_builder_test.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2954,7 +2954,9 @@ TEST_P(FlatExprBuilderVariadicLogicalTest, Evaluate) {
29542954
insert_value("c", test_case.c_val);
29552955

29562956
if (!unknown_patterns.empty()) {
2957-
activation.set_unknown_attribute_patterns(std::move(unknown_patterns));
2957+
ASSERT_THAT(
2958+
activation.SetUnknownAttributePatterns(std::move(unknown_patterns)),
2959+
IsOk());
29582960
}
29592961

29602962
ASSERT_OK_AND_ASSIGN(CelValue result, cel_expr->Evaluate(activation, &arena));

‎eval/eval/BUILD‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,6 @@ cc_test(
418418
deps = [
419419
":attribute_trail",
420420
":direct_expression_step",
421-
":equality_steps",
422421
":evaluator_core",
423422
"//base:attributes",
424423
"//common:value",
@@ -452,7 +451,6 @@ cc_test(
452451
":direct_expression_step",
453452
":evaluator_core",
454453
":expression_step_base",
455-
":ident_step",
456454
"//base:data",
457455
"//common:expr",
458456
"//common:value",
@@ -469,7 +467,6 @@ cc_test(
469467
"//runtime:runtime_options",
470468
"//runtime/internal:runtime_env_testing",
471469
"//runtime/internal:runtime_type_provider",
472-
"@com_google_absl//absl/memory",
473470
"@com_google_absl//absl/status",
474471
"@com_google_absl//absl/status:statusor",
475472
"@com_google_absl//absl/strings",
@@ -519,7 +516,6 @@ cc_test(
519516
":container_access_step",
520517
":direct_expression_step",
521518
":evaluator_core",
522-
":ident_step",
523519
"//base:builtins",
524520
"//base:data",
525521
"//common:ast",
@@ -540,6 +536,7 @@ cc_test(
540536
"//runtime/internal:runtime_env",
541537
"//runtime/internal:runtime_env_testing",
542538
"@com_google_absl//absl/base:nullability",
539+
"@com_google_absl//absl/log:absl_check",
543540
"@com_google_absl//absl/status",
544541
"@com_google_cel_spec//proto/cel/expr:syntax_cc_proto",
545542
"@com_google_protobuf//:protobuf",
@@ -608,8 +605,6 @@ cc_test(
608605
":const_value_step",
609606
":direct_expression_step",
610607
":evaluator_core",
611-
":function_step",
612-
":ident_step",
613608
"//base:builtins",
614609
"//base:data",
615610
"//common:constant",

‎eval/eval/comprehension_step_test.cc‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,13 @@ TEST_F(ListKeysStepTest, MapPartiallyUnknown) {
113113
(*value.mutable_fields())["key3"].set_number_value(3.0);
114114

115115
activation.InsertValue("var", CelProtoWrapper::CreateMessage(&value, &arena));
116-
activation.set_unknown_attribute_patterns({CelAttributePattern(
117-
"var",
118-
{CreateCelAttributeQualifierPattern(CelValue::CreateStringView("key2")),
119-
CreateCelAttributeQualifierPattern(CelValue::CreateStringView("foo")),
120-
CelAttributeQualifierPattern::CreateWildcard()})});
116+
ASSERT_THAT(activation.SetUnknownAttributePatterns({CelAttributePattern(
117+
"var", {CreateCelAttributeQualifierPattern(
118+
CelValue::CreateStringView("key2")),
119+
CreateCelAttributeQualifierPattern(
120+
CelValue::CreateStringView("foo")),
121+
CelAttributeQualifierPattern::CreateWildcard()})}),
122+
IsOk());
121123

122124
auto eval_result = expression->Evaluate(activation, &arena);
123125

@@ -171,7 +173,9 @@ TEST_F(ListKeysStepTest, UnknownSetPassedThrough) {
171173
Activation activation;
172174
Arena arena;
173175

174-
activation.set_unknown_attribute_patterns({CelAttributePattern("var", {})});
176+
ASSERT_THAT(
177+
activation.SetUnknownAttributePatterns({CelAttributePattern("var", {})}),
178+
IsOk());
175179

176180
auto eval_result = expression->Evaluate(activation, &arena);
177181

‎eval/eval/container_access_step_test.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "cel/expr/syntax.pb.h"
1010
#include "google/protobuf/struct.pb.h"
1111
#include "absl/base/nullability.h"
12+
#include "absl/log/absl_check.h"
1213
#include "absl/status/status.h"
1314
#include "base/builtins.h"
1415
#include "base/type_provider.h"
@@ -101,7 +102,7 @@ CelValue EvaluateAttributeHelper(
101102
activation.InsertValue("container", container);
102103
activation.InsertValue("key", key);
103104

104-
activation.set_unknown_attribute_patterns(patterns);
105+
ABSL_CHECK_OK(activation.SetUnknownAttributePatterns(patterns)); // Crash OK
105106
auto result = cel_expr.Evaluate(activation, arena);
106107
return *result;
107108
}

‎eval/eval/create_list_step_test.cc‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -423,8 +423,10 @@ TEST(CreateDirectListStep, MissingAttribute) {
423423
cel::RuntimeOptions options;
424424
options.enable_missing_attribute_errors = true;
425425

426-
activation.SetMissingPatterns({cel::AttributePattern(
427-
"var1", {cel::AttributeQualifierPattern::OfString("field1")})});
426+
ASSERT_THAT(
427+
activation.SetMissingPatterns({cel::AttributePattern(
428+
"var1", {cel::AttributeQualifierPattern::OfString("field1")})}),
429+
IsOk());
428430

429431
ExecutionFrameBase frame(activation, options, type_provider,
430432
cel::internal::GetTestingDescriptorPool(),
@@ -519,8 +521,10 @@ TEST(CreateDirectListStep, PartialUnknown) {
519521
cel::Activation activation;
520522
cel::RuntimeOptions options;
521523
options.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly;
522-
activation.SetUnknownPatterns({cel::AttributePattern(
523-
"var1", {cel::AttributeQualifierPattern::OfString("field1")})});
524+
ASSERT_THAT(
525+
activation.SetUnknownPatterns({cel::AttributePattern(
526+
"var1", {cel::AttributeQualifierPattern::OfString("field1")})}),
527+
IsOk());
524528

525529
ExecutionFrameBase frame(activation, options, type_provider,
526530
cel::internal::GetTestingDescriptorPool(),

‎eval/eval/equality_steps_test.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,9 @@ TEST(RecursiveTest, PartialAttrUnknown) {
9494
std::make_unique<ValueStep>(IntValue(1), cel::Attribute("foo")),
9595
std::make_unique<ValueStep>(IntValue(2)), false, -1);
9696

97-
activation.SetUnknownPatterns({cel::AttributePattern(
98-
"foo", {cel::AttributeQualifierPattern::OfString("bar")})});
97+
ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern(
98+
"foo", {cel::AttributeQualifierPattern::OfString("bar")})}),
99+
IsOk());
99100

100101
ExecutionFrameBase frame(activation, opts, type_provider,
101102
cel::internal::GetTestingDescriptorPool(),
@@ -120,8 +121,9 @@ TEST(RecursiveTest, PartialAttrUnknownDisabled) {
120121
std::make_unique<ValueStep>(IntValue(1), cel::Attribute("foo")),
121122
std::make_unique<ValueStep>(IntValue(2)), false, -1);
122123

123-
activation.SetUnknownPatterns({cel::AttributePattern(
124-
"foo", {cel::AttributeQualifierPattern::OfString("bar")})});
124+
ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern(
125+
"foo", {cel::AttributeQualifierPattern::OfString("bar")})}),
126+
IsOk());
125127
ExecutionFrameBase frame(activation, opts, type_provider,
126128
cel::internal::GetTestingDescriptorPool(),
127129
cel::internal::GetTestingMessageFactory(), &arena);
@@ -154,8 +156,9 @@ TEST(IterativeTest, PartialAttrUnknown) {
154156
std::make_unique<ValueStep>(IntValue(2))));
155157
steps.push_back(ExpressionStep::MakeFastEqualStep());
156158

157-
activation.SetUnknownPatterns({cel::AttributePattern(
158-
"foo", {cel::AttributeQualifierPattern::OfString("bar")})});
159+
ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern(
160+
"foo", {cel::AttributeQualifierPattern::OfString("bar")})}),
161+
IsOk());
159162

160163
ExecutionFrame frame(steps, activation, opts, state);
161164

@@ -185,8 +188,9 @@ TEST(IterativeTest, PartialAttrUnknownDisabled) {
185188
std::make_unique<ValueStep>(IntValue(2))));
186189
steps.push_back(ExpressionStep::MakeFastEqualStep());
187190

188-
activation.SetUnknownPatterns({cel::AttributePattern(
189-
"foo", {cel::AttributeQualifierPattern::OfString("bar")})});
191+
ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern(
192+
"foo", {cel::AttributeQualifierPattern::OfString("bar")})}),
193+
IsOk());
190194
ExecutionFrame frame(steps, activation, opts, state);
191195

192196
ASSERT_OK_AND_ASSIGN(Value result, frame.Evaluate());

‎eval/eval/function_step_test.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -693,7 +693,7 @@ TEST_P(FunctionStepTestUnknowns, PartialUnknownHandlingTest) {
693693
// Set attribute pattern that marks attribute "param[true]" as unknown.
694694
// It should result in "param" being handled as partially unknown, which is
695695
// is handled as fully unknown when used as function input argument.
696-
activation.set_unknown_attribute_patterns({pattern});
696+
ASSERT_THAT(activation.SetUnknownAttributePatterns({pattern}), IsOk());
697697

698698
ASSERT_OK_AND_ASSIGN(CelValue value, impl->Evaluate(activation, &arena));
699699
ASSERT_TRUE(value.IsUnknownSet());

0 commit comments

Comments
 (0)