From 966b0cf4238191c91c291aa0a9b60571231f2c07 Mon Sep 17 00:00:00 2001 From: Sokwhan Huh Date: Wed, 6 May 2026 15:47:37 -0700 Subject: [PATCH] Add support for running policy error cases in conformance tests PiperOrigin-RevId: 911591805 --- .../dev/cel/conformance/policy/BUILD.bazel | 2 + .../policy/PolicyConformanceTest.java | 16 +++++++- .../policy/PolicyConformanceTestRunner.java | 37 +++++++++++++++++-- .../java/dev/cel/policy/RuleComposer.java | 4 +- .../expected_errors.baseline | 4 +- .../expected_errors.baseline | 2 +- 6 files changed, 55 insertions(+), 10 deletions(-) diff --git a/conformance/src/test/java/dev/cel/conformance/policy/BUILD.bazel b/conformance/src/test/java/dev/cel/conformance/policy/BUILD.bazel index 27853f29b..0326b6f15 100644 --- a/conformance/src/test/java/dev/cel/conformance/policy/BUILD.bazel +++ b/conformance/src/test/java/dev/cel/conformance/policy/BUILD.bazel @@ -11,8 +11,10 @@ java_library( srcs = glob(["*.java"]), deps = [ "//:auto_value", + "//:java_truth", "//bundle:cel", "//policy:parser_factory", + "//policy:validation_exception", "//policy/testing:k8s_test_tag_handler", "//runtime:function_binding", "//testing/testrunner:cel_expression_source", diff --git a/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTest.java b/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTest.java index 4f1c643c2..d7851bb72 100644 --- a/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTest.java +++ b/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTest.java @@ -14,11 +14,14 @@ package dev.cel.conformance.policy; +import static com.google.common.truth.Truth.assertThat; + import com.google.protobuf.Struct; import dev.cel.bundle.Cel; import dev.cel.bundle.CelFactory; import dev.cel.expr.conformance.proto3.TestAllTypes; import dev.cel.policy.CelPolicyParserFactory; +import dev.cel.policy.CelPolicyValidationException; import dev.cel.policy.testing.K8sTagHandler; import dev.cel.runtime.CelFunctionBinding; import dev.cel.testing.testrunner.CelExpressionSource; @@ -28,6 +31,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Locale; import org.junit.runners.model.Statement; /** Statement representing a single CEL policy conformance test case. */ @@ -95,6 +99,16 @@ public void evaluate() throws Throwable { contextBuilder.setConfigFile(textprotoConfigPath.toString()); } - TestRunnerLibrary.runTest(testCase, contextBuilder.build()); + try { + TestRunnerLibrary.runTest(testCase, contextBuilder.build()); + } catch (CelPolicyValidationException e) { + if (testCase.output().kind() == CelTestCase.Output.Kind.EVAL_ERROR) { + String expectedError = testCase.output().evalError().get(0).toString(); + assertThat(e.getMessage().toLowerCase(Locale.US)) + .contains(expectedError.toLowerCase(Locale.US)); + } else { + throw e; + } + } } } diff --git a/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTestRunner.java b/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTestRunner.java index 6d1d86e47..62812b124 100644 --- a/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTestRunner.java +++ b/conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTestRunner.java @@ -44,6 +44,7 @@ public final class PolicyConformanceTestRunner extends ParentRunner discoverTestDirs(String testdataDir) { if (!dir.exists() || !dir.isDirectory()) { return ImmutableList.of(); } - String[] directories = dir.list((current, name) -> new File(current, name).isDirectory()); - if (directories == null) { + File[] topLevelDirs = dir.listFiles(File::isDirectory); + if (topLevelDirs == null) { return ImmutableList.of(); } - Arrays.sort(directories); - return ImmutableList.copyOf(directories); + + ImmutableList.Builder testDirsBuilder = ImmutableList.builder(); + Arrays.sort(topLevelDirs); + for (File topLevelDir : topLevelDirs) { + if (hasTestSuite(topLevelDir)) { + testDirsBuilder.add(topLevelDir.getName()); + continue; + } + + // Check one level deeper to support nested tests like compile_errors/unreachable + File[] subDirs = topLevelDir.listFiles(File::isDirectory); + if (subDirs == null) { + continue; + } + + Arrays.sort(subDirs); + for (File subDir : subDirs) { + if (hasTestSuite(subDir)) { + testDirsBuilder.add(topLevelDir.getName() + "/" + subDir.getName()); + } + } + } + + return testDirsBuilder.build(); + } + + private static boolean hasTestSuite(File dir) { + return (new File(dir, TESTS_YAML_FILE_NAME).exists() + || new File(dir, TESTS_TEXTPROTO_FILE_NAME).exists()) + && new File(dir, POLICY_YAML_FILE_NAME).exists(); } private final ImmutableList tests; diff --git a/policy/src/main/java/dev/cel/policy/RuleComposer.java b/policy/src/main/java/dev/cel/policy/RuleComposer.java index cafef4b7c..5fa0957f5 100644 --- a/policy/src/main/java/dev/cel/policy/RuleComposer.java +++ b/policy/src/main/java/dev/cel/policy/RuleComposer.java @@ -93,7 +93,7 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { assertComposedAstIsValid( cel, output.expr, - "conflicting output types found.", + "incompatible output types found.", matchOutput.sourceId(), lastOutputId); lastOutputId = matchOutput.sourceId(); @@ -115,7 +115,7 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) { cel, output.expr, String.format( - "failed composing the subrule '%s' due to conflicting output types.", + "failed composing the subrule '%s' due to incompatible output types.", matchNestedRule.ruleId().map(ValueString::value).orElse("")), lastOutputId); break; diff --git a/testing/src/test/resources/policy/compose_errors_conflicting_output/expected_errors.baseline b/testing/src/test/resources/policy/compose_errors_conflicting_output/expected_errors.baseline index 3e2624b64..0facbbe2e 100644 --- a/testing/src/test/resources/policy/compose_errors_conflicting_output/expected_errors.baseline +++ b/testing/src/test/resources/policy/compose_errors_conflicting_output/expected_errors.baseline @@ -1,6 +1,6 @@ -ERROR: compose_errors_conflicting_output/policy.yaml:22:14: conflicting output types found. +ERROR: compose_errors_conflicting_output/policy.yaml:22:14: incompatible output types found. | output: "false" | .............^ -ERROR: compose_errors_conflicting_output/policy.yaml:23:14: conflicting output types found. +ERROR: compose_errors_conflicting_output/policy.yaml:23:14: incompatible output types found. | - output: "{'banned': true}" | .............^ \ No newline at end of file diff --git a/testing/src/test/resources/policy/compose_errors_conflicting_subrule/expected_errors.baseline b/testing/src/test/resources/policy/compose_errors_conflicting_subrule/expected_errors.baseline index 559d62e1d..92ddff311 100644 --- a/testing/src/test/resources/policy/compose_errors_conflicting_subrule/expected_errors.baseline +++ b/testing/src/test/resources/policy/compose_errors_conflicting_subrule/expected_errors.baseline @@ -1,3 +1,3 @@ -ERROR: compose_errors_conflicting_subrule/policy.yaml:36:14: failed composing the subrule 'banned regions' due to conflicting output types. +ERROR: compose_errors_conflicting_subrule/policy.yaml:36:14: failed composing the subrule 'banned regions' due to incompatible output types. | output: "{'banned': false}" | .............^ \ No newline at end of file