Skip to content

Refactor test runner to accept required descriptors at the callsite, introduce BindingTransformer #993

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor test runner to accept required descriptors at the callsite, introduce BindingTransformer #993

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 23 additions & 7 deletions conformance/src/test/java/dev/cel/conformance/ConformanceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import static dev.cel.testing.utils.ExprValueUtils.DEFAULT_EXTENSION_REGISTRY;
import static dev.cel.testing.utils.ExprValueUtils.DEFAULT_TYPE_REGISTRY;
import static dev.cel.testing.utils.ExprValueUtils.fromValue;
import static dev.cel.testing.utils.ExprValueUtils.toExprValue;

Expand All @@ -29,6 +27,8 @@
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.protobuf.ExtensionRegistry;
import com.google.protobuf.TypeRegistry;
import dev.cel.checker.CelChecker;
import dev.cel.common.CelContainer;
import dev.cel.common.CelOptions;
Expand Down Expand Up @@ -84,6 +84,21 @@ public final class ConformanceTest extends Statement {
CelExtensions.strings(),
CelOptionalLibrary.INSTANCE);

static final TypeRegistry CONFORMANCE_TYPE_REGISTRY =
TypeRegistry.newBuilder()
.add(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor())
.add(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor())
.build();

static final ExtensionRegistry CONFORMANCE_EXTENSION_REGISTRY =
createConformanceExtensionRegistry();

private static ExtensionRegistry createConformanceExtensionRegistry() {
ExtensionRegistry extensionRegistry = ExtensionRegistry.newInstance();
dev.cel.expr.conformance.proto2.TestAllTypesExtensions.registerAllExtensions(extensionRegistry);
return extensionRegistry;
}

private static final CelParser PARSER_WITH_MACROS =
CelParserFactory.standardCelParserBuilder()
.setOptions(OPTIONS)
Expand All @@ -106,7 +121,7 @@ private static CelChecker getChecker(SimpleTest test) throws Exception {
ImmutableList.Builder<Decl> decls =
ImmutableList.builderWithExpectedSize(test.getTypeEnvCount());
for (dev.cel.expr.Decl decl : test.getTypeEnvList()) {
decls.add(Decl.parseFrom(decl.toByteArray(), DEFAULT_EXTENSION_REGISTRY));
decls.add(Decl.parseFrom(decl.toByteArray(), CONFORMANCE_EXTENSION_REGISTRY));
}
return CelCompilerFactory.standardCelCheckerBuilder()
.setOptions(OPTIONS)
Expand All @@ -127,7 +142,7 @@ private static CelRuntime getRuntime(SimpleTest test, boolean usePlanner) {
// CEL-Internal-2
.setOptions(OPTIONS)
.addLibraries(CANONICAL_RUNTIME_EXTENSIONS)
.setExtensionRegistry(DEFAULT_EXTENSION_REGISTRY)
.setExtensionRegistry(CONFORMANCE_EXTENSION_REGISTRY)
.addMessageTypes(dev.cel.expr.conformance.proto2.TestAllTypes.getDescriptor())
.addMessageTypes(dev.cel.expr.conformance.proto3.TestAllTypes.getDescriptor())
.addFileTypes(dev.cel.expr.conformance.proto2.TestAllTypesExtensions.getDescriptor());
Expand All @@ -151,7 +166,8 @@ private static ImmutableMap<String, Object> getBindings(SimpleTest test) throws
private static Object fromExprValue(ExprValue value) throws Exception {
switch (value.getKindCase()) {
case VALUE:
return fromValue(value.getValue());
return fromValue(
value.getValue(), CONFORMANCE_TYPE_REGISTRY, CONFORMANCE_EXTENSION_REGISTRY);
default:
throw new IllegalArgumentException(
String.format("Unexpected binding value kind: %s", value.getKindCase()));
Expand Down Expand Up @@ -224,7 +240,7 @@ public void evaluate() throws Throwable {
assertThat(result)
.ignoringRepeatedFieldOrderOfFieldDescriptors(
MapValue.getDescriptor().findFieldByName("entries"))
.unpackingAnyUsing(DEFAULT_TYPE_REGISTRY, DEFAULT_EXTENSION_REGISTRY)
.unpackingAnyUsing(CONFORMANCE_TYPE_REGISTRY, CONFORMANCE_EXTENSION_REGISTRY)
.isEqualTo(ExprValue.newBuilder().setValue(test.getValue()).build());
break;
case EVAL_ERROR:
Expand All @@ -237,7 +253,7 @@ public void evaluate() throws Throwable {
assertThat(result)
.ignoringRepeatedFieldOrderOfFieldDescriptors(
MapValue.getDescriptor().findFieldByName("entries"))
.unpackingAnyUsing(DEFAULT_TYPE_REGISTRY, DEFAULT_EXTENSION_REGISTRY)
.unpackingAnyUsing(CONFORMANCE_TYPE_REGISTRY, CONFORMANCE_EXTENSION_REGISTRY)
.isEqualTo(ExprValue.newBuilder().setValue(test.getTypedResult().getResult()).build());
assertThat(resultType).isEqualTo(test.getTypedResult().getDeducedType());
break;
Expand Down
9 changes: 5 additions & 4 deletions conformance/src/test/java/dev/cel/conformance/ConformanceTestRunner.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@

package dev.cel.conformance;

import static dev.cel.testing.utils.ExprValueUtils.DEFAULT_EXTENSION_REGISTRY;
import static dev.cel.testing.utils.ExprValueUtils.DEFAULT_TYPE_REGISTRY;
import static dev.cel.conformance.ConformanceTest.CONFORMANCE_EXTENSION_REGISTRY;

import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
Expand Down Expand Up @@ -50,14 +49,16 @@ private static ImmutableSortedMap<String, SimpleTestFile> loadTestFiles() {
SPLITTER.splitToList(System.getProperty("dev.cel.conformance.ConformanceTests.tests"));
try {
TextFormat.Parser parser =
TextFormat.Parser.newBuilder().setTypeRegistry(DEFAULT_TYPE_REGISTRY).build();
TextFormat.Parser.newBuilder()
.setTypeRegistry(ConformanceTest.CONFORMANCE_TYPE_REGISTRY)
.build();
ImmutableSortedMap.Builder<String, SimpleTestFile> testFiles =
ImmutableSortedMap.naturalOrder();
for (String testPath : testPaths) {
SimpleTestFile.Builder fileBuilder = SimpleTestFile.newBuilder();
try (BufferedReader input =
Files.newBufferedReader(Paths.get(testPath), StandardCharsets.UTF_8)) {
parser.merge(input, DEFAULT_EXTENSION_REGISTRY, fileBuilder);
parser.merge(input, CONFORMANCE_EXTENSION_REGISTRY, fileBuilder);
}
SimpleTestFile testFile = fileBuilder.build();
testFiles.put(testFile.getName(), testFile);
Expand Down
7 changes: 7 additions & 0 deletions testing/src/main/java/dev/cel/testing/testrunner/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ java_library(
"//bundle:environment",
"//bundle:environment_yaml_parser",
"//common:cel_ast",
"//common:cel_descriptor_util",
"//common:compiler_common",
"//common:options",
"//common:proto_ast",
Expand Down Expand Up @@ -134,6 +135,7 @@ java_library(
":cel_test_suite",
":cel_test_suite_exception",
"//common:compiler_common",
"//common/annotations",
"//common/formats:file_source",
"//common/formats:parser_context",
"//common/formats:yaml_helper",
Expand Down Expand Up @@ -163,10 +165,14 @@ java_library(
":result_matcher",
"//:auto_value",
"//bundle:cel",
"//common:cel_descriptor_util",
"//common:options",
"//policy:parser",
"//runtime",
"//testing/testrunner:proto_descriptor_utils",
"@maven//:com_google_errorprone_error_prone_annotations",
"@maven//:com_google_guava_guava",
"@maven//:com_google_protobuf_protobuf_java",
],
)

Expand Down Expand Up @@ -223,6 +229,7 @@ java_library(
":cel_test_suite",
":cel_test_suite_exception",
":registry_utils",
"//common/annotations",
"@cel_spec//proto/cel/expr:expr_java_proto",
"@cel_spec//proto/cel/expr/conformance/test:suite_java_proto",
"@maven//:com_google_guava_guava",
Expand Down
82 changes: 82 additions & 0 deletions testing/src/main/java/dev/cel/testing/testrunner/CelTestContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,23 @@
package dev.cel.testing.testrunner;

import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import com.google.protobuf.Descriptors.Descriptor;
import com.google.protobuf.Descriptors.FileDescriptor;
import com.google.protobuf.ExtensionRegistry;
import com.google.protobuf.TypeRegistry;
import dev.cel.bundle.Cel;
import dev.cel.bundle.CelFactory;
import dev.cel.common.CelDescriptorUtil;
import dev.cel.common.CelOptions;
import dev.cel.policy.CelPolicyParser;
import dev.cel.runtime.CelLateFunctionBindings;
import dev.cel.testing.utils.ProtoDescriptorUtils;
import java.io.IOException;
import java.util.Arrays;
import java.util.Map;
import java.util.Optional;

Expand Down Expand Up @@ -63,6 +74,19 @@ public abstract class CelTestContext {
*/
public abstract Optional<CelLateFunctionBindings> celLateFunctionBindings();

/** Interface for transforming bindings before evaluation. */
@FunctionalInterface
public interface BindingTransformer {
ImmutableMap<String, Object> transform(ImmutableMap<String, Object> bindings) throws Exception;
}

/**
* The binding transformer for the CEL test.
*
* <p>This transformer is used to transform the bindings before evaluation.
*/
public abstract Optional<BindingTransformer> bindingTransformer();

/**
* The variable bindings for the CEL test.
*
Expand Down Expand Up @@ -99,6 +123,34 @@ public abstract class CelTestContext {
*/
public abstract Optional<String> fileDescriptorSetPath();

abstract ImmutableSet<FileDescriptor> fileTypes();

@Memoized
public Optional<TypeRegistry> typeRegistry() {
if (fileTypes().isEmpty() && !fileDescriptorSetPath().isPresent()) {
return Optional.empty();
}
TypeRegistry.Builder builder = TypeRegistry.newBuilder();
if (!fileTypes().isEmpty()) {
builder.add(
CelDescriptorUtil.getAllDescriptorsFromFileDescriptor(fileTypes())
.messageTypeDescriptors());
}
if (fileDescriptorSetPath().isPresent()) {
try {
builder.add(
ProtoDescriptorUtils.getAllDescriptorsFromJvm(fileDescriptorSetPath().get())
.messageTypeDescriptors());
} catch (IOException e) {
throw new IllegalStateException(
"Failed to load descriptors from path: " + fileDescriptorSetPath().get(), e);
}
}
return Optional.of(builder.build());
}

public abstract Optional<ExtensionRegistry> extensionRegistry();

/** Returns a builder for {@link CelTestContext} with the current instance's values. */
public abstract Builder toBuilder();

Expand All @@ -123,6 +175,8 @@ public abstract static class Builder {
public abstract Builder setCelLateFunctionBindings(
CelLateFunctionBindings celLateFunctionBindings);

public abstract Builder setBindingTransformer(BindingTransformer bindingTransformer);

public abstract Builder setVariableBindings(Map<String, Object> variableBindings);

public abstract Builder setResultMatcher(ResultMatcher resultMatcher);
Expand All @@ -133,6 +187,34 @@ public abstract Builder setCelLateFunctionBindings(

public abstract Builder setFileDescriptorSetPath(String fileDescriptorSetPath);

abstract ImmutableSet.Builder<FileDescriptor> fileTypesBuilder();

@CanIgnoreReturnValue
public Builder addMessageTypes(Descriptor... descriptors) {
return addMessageTypes(Arrays.asList(descriptors));
}

@CanIgnoreReturnValue
public Builder addMessageTypes(Iterable<Descriptor> descriptors) {
for (Descriptor descriptor : descriptors) {
addFileTypes(descriptor.getFile());
}
return this;
}

@CanIgnoreReturnValue
public Builder addFileTypes(FileDescriptor... fileDescriptors) {
return addFileTypes(Arrays.asList(fileDescriptors));
}

@CanIgnoreReturnValue
public Builder addFileTypes(Iterable<FileDescriptor> fileDescriptors) {
fileTypesBuilder().addAll(fileDescriptors);
return this;
}

public abstract Builder setExtensionRegistry(ExtensionRegistry extensionRegistry);

public abstract CelTestContext build();
}
}
32 changes: 25 additions & 7 deletions testing/src/main/java/dev/cel/testing/testrunner/CelTestSuiteTextProtoParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.protobuf.TextFormat;
import com.google.protobuf.TextFormat.ParseException;
import com.google.protobuf.TypeRegistry;
import dev.cel.common.annotations.Internal;
import dev.cel.expr.conformance.test.InputValue;
import dev.cel.expr.conformance.test.TestCase;
import dev.cel.expr.conformance.test.TestSection;
Expand All @@ -35,23 +36,40 @@
/**
* CelTestSuiteTextProtoParser intakes a textproto document that describes the structure of a CEL
* test suite, parses it then creates a {@link CelTestSuite}.
*
* <p>CEL Library Internals. Do Not Use.
*/
final class CelTestSuiteTextProtoParser {
@Internal
public final class CelTestSuiteTextProtoParser {

/** Creates a new instance of {@link CelTestSuiteTextProtoParser}. */
static CelTestSuiteTextProtoParser newInstance() {
public static CelTestSuiteTextProtoParser newInstance() {
return new CelTestSuiteTextProtoParser();
}

CelTestSuite parse(String textProto) throws IOException, CelTestSuiteException {
TestSuite testSuite = parseTestSuite(textProto);
public CelTestSuite parse(String textProto) throws IOException, CelTestSuiteException {
return parse(
textProto, TypeRegistry.getEmptyTypeRegistry(), ExtensionRegistry.getEmptyRegistry());
}

public CelTestSuite parse(String textProto, TypeRegistry customTypeRegistry)
throws IOException, CelTestSuiteException {
return parse(textProto, customTypeRegistry, ExtensionRegistry.getEmptyRegistry());
}

public CelTestSuite parse(
String textProto, TypeRegistry customTypeRegistry, ExtensionRegistry customExtensionRegistry)
throws IOException, CelTestSuiteException {
TestSuite testSuite = parseTestSuite(textProto, customTypeRegistry, customExtensionRegistry);
return parseCelTestSuite(testSuite);
}

private TestSuite parseTestSuite(String textProto) throws IOException {
private TestSuite parseTestSuite(
String textProto, TypeRegistry customTypeRegistry, ExtensionRegistry customExtensionRegistry)
throws IOException {
String fileDescriptorSetPath = System.getProperty("file_descriptor_set_path");
TypeRegistry typeRegistry = TypeRegistry.getEmptyTypeRegistry();
ExtensionRegistry extensionRegistry = ExtensionRegistry.getEmptyRegistry();
TypeRegistry typeRegistry = customTypeRegistry;
ExtensionRegistry extensionRegistry = customExtensionRegistry;
if (fileDescriptorSetPath != null) {
extensionRegistry = RegistryUtils.getExtensionRegistry(fileDescriptorSetPath);
typeRegistry = RegistryUtils.getTypeRegistry(fileDescriptorSetPath);
Expand Down
11 changes: 8 additions & 3 deletions testing/src/main/java/dev/cel/testing/testrunner/CelTestSuiteYamlParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import dev.cel.common.CelIssue;
import dev.cel.common.annotations.Internal;
import dev.cel.common.formats.CelFileSource;
import dev.cel.common.formats.ParserContext;
import dev.cel.common.formats.YamlHelper.YamlNodeType;
Expand All @@ -43,15 +44,18 @@
/**
* CelTestSuiteYamlParser intakes a YAML document that describes the structure of a CEL test suite,
* parses it then creates a {@link CelTestSuite}.
*
* <p>CEL Library Internals. Do Not Use.
*/
final class CelTestSuiteYamlParser {
@Internal
public final class CelTestSuiteYamlParser {

/** Creates a new instance of {@link CelTestSuiteYamlParser}. */
static CelTestSuiteYamlParser newInstance() {
public static CelTestSuiteYamlParser newInstance() {
return new CelTestSuiteYamlParser();
}

CelTestSuite parse(String celTestSuiteYamlContent) throws CelTestSuiteException {
public CelTestSuite parse(String celTestSuiteYamlContent) throws CelTestSuiteException {
return parseYaml(celTestSuiteYamlContent, "<input>");
}

Expand Down Expand Up @@ -110,6 +114,7 @@ private CelTestSuite.Builder parseTestSuite(ParserContext<Node> ctx, Node node)
case "description":
builder.setDescription(newString(ctx, valueNode));
break;
case "section":
case "sections":
builder.setSections(parseSections(ctx, valueNode));
break;
Expand Down
Loading