Skip to content

Commit 462a880

Browse files
jnthntatumcopybara-github
jnthntatum
authored and
copybara-github
committed
Add conformance tests for legacy runtime using checked expressions.
PiperOrigin-RevId: 839841283
1 parent 4eef486 commit 462a880

2 files changed

Lines changed: 120 additions & 89 deletions

File tree

‎conformance/BUILD‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,12 @@ _TESTS_TO_SKIP_LEGACY = [
249249
"block_ext/basic/optional_message",
250250
]
251251

252+
_TESTS_TO_SKIP_CHECKED = [
253+
# block is a post-check optimization that inserts internal variables. The C++ type checker
254+
# needs support for a proper optimizer for this to work.
255+
"block_ext",
256+
]
257+
252258
_TESTS_TO_SKIP_LEGACY_DASHBOARD = [
253259
# Future features for CEL 1.0
254260
# TODO(issues/119): Strong typing support for enums, specified but not implemented.
@@ -280,11 +286,15 @@ gen_conformance_tests(
280286
checked = True,
281287
data = _ALL_TESTS,
282288
modern = True,
283-
skip_tests = _TESTS_TO_SKIP_MODERN + [
284-
# block is a post-check optimization that inserts internal variables. The C++ type checker
285-
# needs support for a proper optimizer for this to work.
286-
"block_ext",
287-
],
289+
skip_tests = _TESTS_TO_SKIP_MODERN + _TESTS_TO_SKIP_CHECKED,
290+
)
291+
292+
gen_conformance_tests(
293+
name = "conformance_legacy_checked",
294+
checked = True,
295+
data = _ALL_TESTS,
296+
modern = False,
297+
skip_tests = _TESTS_TO_SKIP_LEGACY + _TESTS_TO_SKIP_CHECKED,
288298
)
289299

290300
# Generates a bunch of `cc_test` whose names follow the pattern

‎conformance/service.cc‎

Lines changed: 105 additions & 84 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,83 @@ absl::Status LegacyParse(const conformance::v1alpha1::ParseRequest& request,
260260
return absl::OkStatus();
261261
}
262262

263+
absl::Status CheckImpl(google::protobuf::Arena* arena,
264+
const conformance::v1alpha1::CheckRequest& request,
265+
conformance::v1alpha1::CheckResponse& response) {
266+
cel::expr::ParsedExpr parsed_expr;
267+
268+
ABSL_CHECK(ConvertWireCompatProto(request.parsed_expr(), // Crash OK
269+
&parsed_expr));
270+
271+
CEL_ASSIGN_OR_RETURN(std::unique_ptr<cel::Ast> ast,
272+
cel::CreateAstFromParsedExpr(parsed_expr));
273+
274+
absl::string_view location = parsed_expr.source_info().location();
275+
std::unique_ptr<cel::Source> source;
276+
if (absl::StartsWith(location, "Source: ")) {
277+
location = absl::StripPrefix(location, "Source: ");
278+
CEL_ASSIGN_OR_RETURN(source, cel::NewSource(location));
279+
}
280+
281+
CEL_ASSIGN_OR_RETURN(
282+
std::unique_ptr<cel::TypeCheckerBuilder> builder,
283+
cel::CreateTypeCheckerBuilder(google::protobuf::DescriptorPool::generated_pool()));
284+
285+
if (!request.no_std_env()) {
286+
CEL_RETURN_IF_ERROR(builder->AddLibrary(cel::StandardCheckerLibrary()));
287+
CEL_RETURN_IF_ERROR(builder->AddLibrary(cel::OptionalCheckerLibrary()));
288+
CEL_RETURN_IF_ERROR(
289+
builder->AddLibrary(cel::extensions::StringsCheckerLibrary()));
290+
CEL_RETURN_IF_ERROR(
291+
builder->AddLibrary(cel::extensions::MathCheckerLibrary()));
292+
CEL_RETURN_IF_ERROR(
293+
builder->AddLibrary(cel::extensions::EncodersCheckerLibrary()));
294+
}
295+
296+
for (const auto& decl : request.type_env()) {
297+
const auto& name = decl.name();
298+
if (decl.has_function()) {
299+
CEL_ASSIGN_OR_RETURN(
300+
auto fn_decl, cel::FunctionDeclFromV1Alpha1Proto(
301+
name, decl.function(),
302+
google::protobuf::DescriptorPool::generated_pool(), arena));
303+
CEL_RETURN_IF_ERROR(builder->AddFunction(std::move(fn_decl)));
304+
} else if (decl.has_ident()) {
305+
CEL_ASSIGN_OR_RETURN(
306+
auto var_decl, cel::VariableDeclFromV1Alpha1Proto(
307+
name, decl.ident(),
308+
google::protobuf::DescriptorPool::generated_pool(), arena));
309+
CEL_RETURN_IF_ERROR(builder->AddVariable(std::move(var_decl)));
310+
}
311+
}
312+
builder->set_container(request.container());
313+
314+
CEL_ASSIGN_OR_RETURN(auto checker, std::move(*builder).Build());
315+
316+
CEL_ASSIGN_OR_RETURN(auto validation_result, checker->Check(std::move(ast)));
317+
318+
for (const auto& checker_issue : validation_result.GetIssues()) {
319+
auto* issue = response.add_issues();
320+
issue->set_code(ToGrpcCode(absl::StatusCode::kInvalidArgument));
321+
if (source) {
322+
issue->set_message(checker_issue.ToDisplayString(*source));
323+
} else {
324+
issue->set_message(checker_issue.message());
325+
}
326+
}
327+
328+
const cel::Ast* checked_ast = validation_result.GetAst();
329+
if (!validation_result.IsValid() || checked_ast == nullptr) {
330+
return absl::OkStatus();
331+
}
332+
cel::expr::CheckedExpr pb_checked_ast;
333+
CEL_RETURN_IF_ERROR(
334+
cel::AstToCheckedExpr(*validation_result.GetAst(), &pb_checked_ast));
335+
ABSL_CHECK(ConvertWireCompatProto(pb_checked_ast, // Crash OK
336+
response.mutable_checked_expr()));
337+
return absl::OkStatus();
338+
}
339+
263340
class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
264341
public:
265342
static absl::StatusOr<std::unique_ptr<LegacyConformanceServiceImpl>> Create(
@@ -351,9 +428,13 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
351428

352429
void Check(const conformance::v1alpha1::CheckRequest& request,
353430
conformance::v1alpha1::CheckResponse& response) override {
354-
auto issue = response.add_issues();
355-
issue->set_message("Check is not supported");
356-
issue->set_code(google::rpc::Code::UNIMPLEMENTED);
431+
google::protobuf::Arena arena;
432+
auto status = CheckImpl(&arena, request, response);
433+
if (!status.ok()) {
434+
auto* issue = response.add_issues();
435+
issue->set_code(ToGrpcCode(status.code()));
436+
issue->set_message(status.message());
437+
}
357438
}
358439

359440
absl::Status Eval(const conformance::v1alpha1::EvalRequest& request,
@@ -362,8 +443,26 @@ class LegacyConformanceServiceImpl : public ConformanceServiceInterface {
362443
cel::expr::SourceInfo source_info;
363444
cel::expr::Expr expr = ExtractExpr(request);
364445
builder_->set_container(request.container());
365-
auto cel_expression_status =
366-
builder_->CreateExpression(&expr, &source_info);
446+
absl::StatusOr<std::unique_ptr<CelExpression>> cel_expression_status =
447+
absl::InternalError(
448+
"no expression provided in ConformanceService::Eval");
449+
450+
if (request.has_parsed_expr()) {
451+
cel::expr::ParsedExpr parsed_expr;
452+
if (!ConvertWireCompatProto(request.parsed_expr(), &parsed_expr)) {
453+
return absl::InternalError(
454+
"failed to convert versioned ParsedExpr to unversioned");
455+
}
456+
cel_expression_status = builder_->CreateExpression(
457+
&parsed_expr.expr(), &parsed_expr.source_info());
458+
} else if (request.has_checked_expr()) {
459+
cel::expr::CheckedExpr checked_expr;
460+
if (!ConvertWireCompatProto(request.checked_expr(), &checked_expr)) {
461+
return absl::InternalError(
462+
"failed to convert versioned CheckedExpr to unversioned");
463+
}
464+
cel_expression_status = builder_->CreateExpression(&checked_expr);
465+
}
367466

368467
if (!cel_expression_status.ok()) {
369468
return absl::InternalError(cel_expression_status.status().ToString(
@@ -527,7 +626,7 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
527626
void Check(const conformance::v1alpha1::CheckRequest& request,
528627
conformance::v1alpha1::CheckResponse& response) override {
529628
google::protobuf::Arena arena;
530-
auto status = DoCheck(&arena, request, response);
629+
auto status = CheckImpl(&arena, request, response);
531630
if (!status.ok()) {
532631
auto* issue = response.add_issues();
533632
issue->set_code(ToGrpcCode(status.code()));
@@ -609,84 +708,6 @@ class ModernConformanceServiceImpl : public ConformanceServiceInterface {
609708
bool enable_optimizations)
610709
: options_(options), enable_optimizations_(enable_optimizations) {}
611710

612-
static absl::Status DoCheck(
613-
google::protobuf::Arena* arena, const conformance::v1alpha1::CheckRequest& request,
614-
conformance::v1alpha1::CheckResponse& response) {
615-
cel::expr::ParsedExpr parsed_expr;
616-
617-
ABSL_CHECK(ConvertWireCompatProto(request.parsed_expr(), // Crash OK
618-
&parsed_expr));
619-
620-
CEL_ASSIGN_OR_RETURN(std::unique_ptr<cel::Ast> ast,
621-
cel::CreateAstFromParsedExpr(parsed_expr));
622-
623-
absl::string_view location = parsed_expr.source_info().location();
624-
std::unique_ptr<cel::Source> source;
625-
if (absl::StartsWith(location, "Source: ")) {
626-
location = absl::StripPrefix(location, "Source: ");
627-
CEL_ASSIGN_OR_RETURN(source, cel::NewSource(location));
628-
}
629-
630-
CEL_ASSIGN_OR_RETURN(std::unique_ptr<cel::TypeCheckerBuilder> builder,
631-
cel::CreateTypeCheckerBuilder(
632-
google::protobuf::DescriptorPool::generated_pool()));
633-
634-
if (!request.no_std_env()) {
635-
CEL_RETURN_IF_ERROR(builder->AddLibrary(cel::StandardCheckerLibrary()));
636-
CEL_RETURN_IF_ERROR(builder->AddLibrary(cel::OptionalCheckerLibrary()));
637-
CEL_RETURN_IF_ERROR(
638-
builder->AddLibrary(cel::extensions::StringsCheckerLibrary()));
639-
CEL_RETURN_IF_ERROR(
640-
builder->AddLibrary(cel::extensions::MathCheckerLibrary()));
641-
CEL_RETURN_IF_ERROR(
642-
builder->AddLibrary(cel::extensions::EncodersCheckerLibrary()));
643-
}
644-
645-
for (const auto& decl : request.type_env()) {
646-
const auto& name = decl.name();
647-
if (decl.has_function()) {
648-
CEL_ASSIGN_OR_RETURN(
649-
auto fn_decl, cel::FunctionDeclFromV1Alpha1Proto(
650-
name, decl.function(),
651-
google::protobuf::DescriptorPool::generated_pool(), arena));
652-
CEL_RETURN_IF_ERROR(builder->AddFunction(std::move(fn_decl)));
653-
} else if (decl.has_ident()) {
654-
CEL_ASSIGN_OR_RETURN(
655-
auto var_decl,
656-
cel::VariableDeclFromV1Alpha1Proto(
657-
name, decl.ident(), google::protobuf::DescriptorPool::generated_pool(),
658-
arena));
659-
CEL_RETURN_IF_ERROR(builder->AddVariable(std::move(var_decl)));
660-
}
661-
}
662-
builder->set_container(request.container());
663-
664-
CEL_ASSIGN_OR_RETURN(auto checker, std::move(*builder).Build());
665-
666-
CEL_ASSIGN_OR_RETURN(auto validation_result,
667-
checker->Check(std::move(ast)));
668-
669-
for (const auto& checker_issue : validation_result.GetIssues()) {
670-
auto* issue = response.add_issues();
671-
issue->set_code(ToGrpcCode(absl::StatusCode::kInvalidArgument));
672-
if (source) {
673-
issue->set_message(checker_issue.ToDisplayString(*source));
674-
} else {
675-
issue->set_message(checker_issue.message());
676-
}
677-
}
678-
679-
const cel::Ast* checked_ast = validation_result.GetAst();
680-
if (!validation_result.IsValid() || checked_ast == nullptr) {
681-
return absl::OkStatus();
682-
}
683-
cel::expr::CheckedExpr pb_checked_ast;
684-
CEL_RETURN_IF_ERROR(
685-
cel::AstToCheckedExpr(*validation_result.GetAst(), &pb_checked_ast));
686-
ABSL_CHECK(ConvertWireCompatProto(pb_checked_ast, // Crash OK
687-
response.mutable_checked_expr()));
688-
return absl::OkStatus();
689-
}
690711

691712
static absl::StatusOr<std::unique_ptr<cel::TraceableProgram>> Plan(
692713
const cel::Runtime& runtime,

0 commit comments

Comments
 (0)