Skip to content

Commit 573aaba

Browse files
l46kokcopybara-github
authored andcommitted
Add support for running policy error cases in conformance tests
PiperOrigin-RevId: 911070494
1 parent f89d7e5 commit 573aaba

6 files changed

Lines changed: 55 additions & 10 deletions

File tree

conformance/src/test/java/dev/cel/conformance/policy/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,10 @@ java_library(
1111
srcs = glob(["*.java"]),
1212
deps = [
1313
"//:auto_value",
14+
"//:java_truth",
1415
"//bundle:cel",
1516
"//policy:parser_factory",
17+
"//policy:validation_exception",
1618
"//policy/testing:k8s_test_tag_handler",
1719
"//runtime:function_binding",
1820
"//testing/testrunner:cel_expression_source",

conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTest.java

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,14 @@
1414

1515
package dev.cel.conformance.policy;
1616

17+
import static com.google.common.truth.Truth.assertThat;
18+
1719
import com.google.protobuf.Struct;
1820
import dev.cel.bundle.Cel;
1921
import dev.cel.bundle.CelFactory;
2022
import dev.cel.expr.conformance.proto3.TestAllTypes;
2123
import dev.cel.policy.CelPolicyParserFactory;
24+
import dev.cel.policy.CelPolicyValidationException;
2225
import dev.cel.policy.testing.K8sTagHandler;
2326
import dev.cel.runtime.CelFunctionBinding;
2427
import dev.cel.testing.testrunner.CelExpressionSource;
@@ -28,6 +31,7 @@
2831
import java.nio.file.Files;
2932
import java.nio.file.Path;
3033
import java.nio.file.Paths;
34+
import java.util.Locale;
3135
import org.junit.runners.model.Statement;
3236

3337
/** Statement representing a single CEL policy conformance test case. */
@@ -95,6 +99,16 @@ public void evaluate() throws Throwable {
9599
contextBuilder.setConfigFile(textprotoConfigPath.toString());
96100
}
97101

98-
TestRunnerLibrary.runTest(testCase, contextBuilder.build());
102+
try {
103+
TestRunnerLibrary.runTest(testCase, contextBuilder.build());
104+
} catch (CelPolicyValidationException e) {
105+
if (testCase.output().kind() == CelTestCase.Output.Kind.EVAL_ERROR) {
106+
String expectedError = testCase.output().evalError().get(0).toString();
107+
assertThat(e.getMessage().toLowerCase(Locale.US))
108+
.contains(expectedError.toLowerCase(Locale.US));
109+
} else {
110+
throw e;
111+
}
112+
}
99113
}
100114
}

conformance/src/test/java/dev/cel/conformance/policy/PolicyConformanceTestRunner.java

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public final class PolicyConformanceTestRunner extends ParentRunner<PolicyConfor
4444
private static final Splitter SPLITTER = Splitter.on(",").omitEmptyStrings();
4545
private static final String TESTS_YAML_FILE_NAME = "tests.yaml";
4646
private static final String TESTS_TEXTPROTO_FILE_NAME = "tests.textproto";
47+
private static final String POLICY_YAML_FILE_NAME = "policy.yaml";
4748
private static final TypeRegistry TYPE_REGISTRY =
4849
TypeRegistry.newBuilder()
4950
.add(Struct.getDescriptor())
@@ -73,12 +74,40 @@ private static ImmutableList<String> discoverTestDirs(String testdataDir) {
7374
if (!dir.exists() || !dir.isDirectory()) {
7475
return ImmutableList.of();
7576
}
76-
String[] directories = dir.list((current, name) -> new File(current, name).isDirectory());
77-
if (directories == null) {
77+
File[] topLevelDirs = dir.listFiles(File::isDirectory);
78+
if (topLevelDirs == null) {
7879
return ImmutableList.of();
7980
}
80-
Arrays.sort(directories);
81-
return ImmutableList.copyOf(directories);
81+
82+
ImmutableList.Builder<String> testDirsBuilder = ImmutableList.builder();
83+
Arrays.sort(topLevelDirs);
84+
for (File topLevelDir : topLevelDirs) {
85+
if (hasTestSuite(topLevelDir)) {
86+
testDirsBuilder.add(topLevelDir.getName());
87+
continue;
88+
}
89+
90+
// Check one level deeper to support nested tests like compile_errors/unreachable
91+
File[] subDirs = topLevelDir.listFiles(File::isDirectory);
92+
if (subDirs == null) {
93+
continue;
94+
}
95+
96+
Arrays.sort(subDirs);
97+
for (File subDir : subDirs) {
98+
if (hasTestSuite(subDir)) {
99+
testDirsBuilder.add(topLevelDir.getName() + "/" + subDir.getName());
100+
}
101+
}
102+
}
103+
104+
return testDirsBuilder.build();
105+
}
106+
107+
private static boolean hasTestSuite(File dir) {
108+
return (new File(dir, TESTS_YAML_FILE_NAME).exists()
109+
|| new File(dir, TESTS_TEXTPROTO_FILE_NAME).exists())
110+
&& new File(dir, POLICY_YAML_FILE_NAME).exists();
82111
}
83112

84113
private final ImmutableList<PolicyConformanceTest> tests;

policy/src/main/java/dev/cel/policy/RuleComposer.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) {
9393
assertComposedAstIsValid(
9494
cel,
9595
output.expr,
96-
"conflicting output types found.",
96+
"incompatible output types found.",
9797
matchOutput.sourceId(),
9898
lastOutputId);
9999
lastOutputId = matchOutput.sourceId();
@@ -115,7 +115,7 @@ private Step optimizeRule(Cel cel, CelCompiledRule compiledRule) {
115115
cel,
116116
output.expr,
117117
String.format(
118-
"failed composing the subrule '%s' due to conflicting output types.",
118+
"failed composing the subrule '%s' due to incompatible output types.",
119119
matchNestedRule.ruleId().map(ValueString::value).orElse("")),
120120
lastOutputId);
121121
break;
Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
ERROR: compose_errors_conflicting_output/policy.yaml:22:14: conflicting output types found.
1+
ERROR: compose_errors_conflicting_output/policy.yaml:22:14: incompatible output types found.
22
| output: "false"
33
| .............^
4-
ERROR: compose_errors_conflicting_output/policy.yaml:23:14: conflicting output types found.
4+
ERROR: compose_errors_conflicting_output/policy.yaml:23:14: incompatible output types found.
55
| - output: "{'banned': true}"
66
| .............^
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
ERROR: compose_errors_conflicting_subrule/policy.yaml:36:14: failed composing the subrule 'banned regions' due to conflicting output types.
1+
ERROR: compose_errors_conflicting_subrule/policy.yaml:36:14: failed composing the subrule 'banned regions' due to incompatible output types.
22
| output: "{'banned': false}"
33
| .............^

0 commit comments

Comments
 (0)