Skip to content

Commit fb930cf

Browse files
jnthntatumcopybara-github
authored andcommitted
Update policy conformance tests to use the bundled format.
PiperOrigin-RevId: 943992143
1 parent a1721f3 commit fb930cf

4 files changed

Lines changed: 132 additions & 90 deletions

File tree

MODULE.bazel

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,9 +98,9 @@ bazel_dep(
9898
version = "0.9.0",
9999
)
100100

101-
_CEL_POLICY_TAG = "ebfb2361f47080af643c14cf4da4c2b551a68740"
101+
_CEL_POLICY_TAG = "4c2c2fd5b8f20da4eb34e3ee82a4ef1b7f023347"
102102

103-
_CEL_POLICY_SHA = "ea69e9c6b7bd5bc37d358148aebd2fcca38bc7c45a23feb635de72338e0327c1"
103+
_CEL_POLICY_SHA = "e2bd943f89a3cab3b6508daf0bc5aa53d00e3e6ea52ef14a1d6f618a2b2909be"
104104

105105
http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
106106

conformance/policy/BUILD

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,39 @@ cc_library(
6464
alwayslink = True,
6565
)
6666

67+
_ALL_TESTS = [
68+
"@cel_policy//conformance:k8s",
69+
"@cel_policy//conformance:limits",
70+
"@cel_policy//conformance:nested_rule",
71+
"@cel_policy//conformance:nested_rule2",
72+
"@cel_policy//conformance:nested_rule3",
73+
"@cel_policy//conformance:nested_rule4",
74+
"@cel_policy//conformance:nested_rule5",
75+
"@cel_policy//conformance:nested_rule6",
76+
"@cel_policy//conformance:nested_rule7",
77+
"@cel_policy//conformance:nested_rules_variable_shadowing",
78+
"@cel_policy//conformance:pb",
79+
"@cel_policy//conformance:required_labels",
80+
"@cel_policy//conformance:restricted_destinations",
81+
"@cel_policy//conformance:unconditional_rules",
82+
"@cel_policy//conformance:unnest",
83+
"@cel_policy//conformance:variable_type_propagation",
84+
"@cel_policy//conformance:compile_error_compose_conflicting_output",
85+
"@cel_policy//conformance:compile_error_compose_conflicting_subrule",
86+
"@cel_policy//conformance:compile_error_unreachable",
87+
"@cel_policy//conformance:compile_error_duplicate_variable",
88+
"@cel_policy//conformance:compile_error_import",
89+
"@cel_policy//conformance:compile_error_incompatible_outputs",
90+
"@cel_policy//conformance:compile_error_syntax",
91+
"@cel_policy//conformance:compile_error_undeclared_reference",
92+
]
93+
6794
cel_policy_conformance_test(
6895
name = "policy_conformance_test",
69-
example = "@cel_policy//conformance:testdata/nested_rule/policy.yaml",
7096
skip_tests = [
7197
# TODO(b/506179116): Fix these.
7298
# Need to add k8s custom yaml parser and mock runtime.
7399
"k8s",
74100
],
75-
test_files = [
76-
"@cel_policy//conformance:testdata",
77-
],
101+
test_files = _ALL_TESTS,
78102
)

conformance/policy/policy_conformance_test.bzl

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,25 @@ This module contains build rules for generating policy conformance test targets.
1818

1919
load("@rules_cc//cc:cc_test.bzl", "cc_test")
2020

21-
def cel_policy_conformance_test(name, test_files, example, skip_tests = [], **kwargs):
21+
def cel_policy_conformance_test(name, test_files, skip_tests = [], **kwargs):
2222
"""Generates a policy conformance test target.
2323
2424
Args:
2525
name: Name of the test target.
2626
test_files: List of targets or files representing the test data.
27-
example: A specific example file from test_files used for runfiles resolution.
2827
skip_tests: List of test cases to skip.
29-
testdata_dir: Path to testdata directory under runfiles.
3028
**kwargs: Additional arguments passed to the underlying cc_test.
3129
"""
3230
args = ["--gunit_fail_if_no_test_linked"]
33-
args.append("--testdata_example='$(rlocationpath {})'".format(example))
31+
paths = ["$(rlocationpath {})".format(f) for f in test_files]
32+
args.append("--test_bundles=\"{}\"".format(",".join(paths)))
3433

3534
if skip_tests:
3635
args.append("--skip_tests=" + ",".join(skip_tests))
3736

3837
cc_test(
3938
name = name,
40-
data = test_files + [example],
39+
data = test_files,
4140
deps = [
4241
"//conformance/policy:policy_conformance_test_lib",
4342
],

conformance/policy/policy_conformance_test.cc

Lines changed: 98 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,12 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15+
#include <cstddef>
1516
#include <iostream>
1617
#include <memory>
1718
#include <string>
1819
#include <utility>
1920
#include <vector>
20-
// NOLINTNEXTLINE(build/c++17) for OSS compatibility
21-
#include <filesystem>
2221

2322
#include "cel/expr/eval.pb.h"
2423
#include "absl/flags/flag.h"
@@ -28,8 +27,8 @@
2827
#include "absl/status/statusor.h"
2928
#include "absl/strings/match.h"
3029
#include "absl/strings/str_cat.h"
31-
#include "absl/strings/str_replace.h"
3230
#include "absl/strings/string_view.h"
31+
#include "absl/strings/strip.h"
3332
#include "common/ast.h"
3433
#include "common/internal/value_conversion.h"
3534
#include "common/source.h"
@@ -61,12 +60,9 @@
6160
#include "google/protobuf/descriptor.h"
6261
#include "google/protobuf/dynamic_message.h"
6362
#include "google/protobuf/message.h"
64-
#include "google/protobuf/text_format.h"
6563

66-
// Use a specific file to handle bazel runfiles resolution correctly. We find
67-
// parent directory named 'testdata' to use as the root of the test cases.
68-
ABSL_FLAG(std::string, testdata_example, "",
69-
"Path to a specific example file.");
64+
ABSL_FLAG(std::vector<std::string>, test_bundles, {},
65+
"Space or comma separated list of test bundle runfiles paths.");
7066
ABSL_FLAG(std::vector<std::string>, skip_tests, {},
7167
"Comma-separated list of tests to skip.");
7268

@@ -78,6 +74,63 @@ using ::cel::expr::conformance::test::TestSuite;
7874
using ::cel::internal::GetSharedTestingDescriptorPool;
7975
using ::testing::HasSubstr;
8076

77+
struct BundleSections {
78+
absl::string_view config_content;
79+
absl::string_view policy_content;
80+
absl::string_view tests_content;
81+
};
82+
83+
absl::string_view TrimDoc(absl::string_view doc) {
84+
absl::ConsumeSuffix(&doc, "\n");
85+
absl::ConsumeSuffix(&doc, "\r");
86+
return doc;
87+
}
88+
89+
absl::StatusOr<BundleSections> ParseYamlBundle(
90+
absl::string_view bundle_content) {
91+
BundleSections sections;
92+
std::vector<absl::string_view> docs;
93+
absl::string_view remaining = bundle_content;
94+
95+
size_t next_line = remaining.find('\n');
96+
while (next_line != absl::string_view::npos) {
97+
if (absl::StartsWith(remaining.substr(next_line), "\n---\r\n")) {
98+
docs.push_back(TrimDoc(remaining.substr(0, next_line)));
99+
remaining = remaining.substr(next_line + 5);
100+
next_line = remaining.find('\n');
101+
continue;
102+
}
103+
104+
if (absl::StartsWith(remaining.substr(next_line), "\n---\n")) {
105+
docs.push_back(TrimDoc(remaining.substr(0, next_line)));
106+
remaining = remaining.substr(next_line + 4);
107+
next_line = remaining.find('\n');
108+
continue;
109+
}
110+
111+
next_line = remaining.find('\n', next_line + 1);
112+
}
113+
114+
if (remaining.empty()) {
115+
return absl::InvalidArgumentError("Empty document in yaml bundle");
116+
}
117+
docs.push_back(remaining);
118+
119+
if (docs.size() == 3) {
120+
sections.config_content = docs[0];
121+
sections.policy_content = docs[1];
122+
sections.tests_content = docs[2];
123+
} else if (docs.size() == 2) {
124+
sections.policy_content = docs[0];
125+
sections.tests_content = docs[1];
126+
} else {
127+
return absl::InvalidArgumentError(
128+
absl::StrCat("Invalid number of sections: ", docs.size()));
129+
}
130+
131+
return sections;
132+
}
133+
81134
// Implementations for extension functions referenced in conformance tests.
82135
cel::Value LocationCode(const cel::StringValue& ip,
83136
const google::protobuf::DescriptorPool* pool,
@@ -447,9 +500,8 @@ class CelPolicyTest : public testing::Test {
447500
bool skip_;
448501
};
449502

450-
451503
absl::Status RegisterTestSuite(
452-
const std::filesystem::path& dir_path, const std::string& suite_name,
504+
absl::string_view suite_name, const BundleSections& sections,
453505
const std::shared_ptr<InputEvaluator>& input_evaluator,
454506
const std::shared_ptr<const google::protobuf::DescriptorPool>& pool,
455507
const std::shared_ptr<google::protobuf::MessageFactory>& message_factory,
@@ -463,28 +515,15 @@ absl::Status RegisterTestSuite(
463515
}
464516
}
465517

466-
std::filesystem::path policy_path = dir_path / "policy.yaml";
467-
std::filesystem::path tests_path = dir_path / "tests.yaml";
468-
bool is_yaml = true;
469-
if (!std::filesystem::exists(tests_path)) {
470-
tests_path = dir_path / "tests.textproto";
471-
is_yaml = false;
472-
}
473-
std::filesystem::path config_path = dir_path / "config.yaml";
474-
475-
if (!std::filesystem::exists(policy_path) ||
476-
!std::filesystem::exists(tests_path)) {
477-
// Not a valid test suite, assume it's a directory we don't care about.
518+
if (sections.policy_content.empty() || sections.tests_content.empty()) {
478519
return absl::OkStatus();
479520
}
480521

481522
// Parse Environment Config
482523
cel::Config config;
483-
if (std::filesystem::exists(config_path)) {
484-
std::string config_content;
485-
CEL_RETURN_IF_ERROR(
486-
cel::internal::GetFileContents(config_path.string(), &config_content));
487-
CEL_ASSIGN_OR_RETURN(config, cel::EnvConfigFromYaml(config_content));
524+
if (!sections.config_content.empty()) {
525+
CEL_ASSIGN_OR_RETURN(
526+
config, cel::EnvConfigFromYaml(std::string(sections.config_content)));
488527
}
489528

490529
// Enable default extensions (optional, bindings) in the config
@@ -537,11 +576,8 @@ absl::Status RegisterTestSuite(
537576
CEL_ASSIGN_OR_RETURN(auto runtime, std::move(runtime_builder).Build());
538577

539578
// Parse Policy
540-
std::string policy_content;
541-
CEL_RETURN_IF_ERROR(
542-
cel::internal::GetFileContents(policy_path.string(), &policy_content));
543579
CEL_ASSIGN_OR_RETURN(auto source,
544-
cel::NewSource(policy_content, "policy.yaml"));
580+
cel::NewSource(sections.policy_content, "policy.yaml"));
545581
auto policy_source = std::make_shared<CelPolicySource>(std::move(source));
546582
CEL_ASSIGN_OR_RETURN(CelPolicyParseResult parse_result,
547583
cel::ParseYamlCelPolicy(policy_source));
@@ -556,25 +592,16 @@ absl::Status RegisterTestSuite(
556592
CEL_ASSIGN_OR_RETURN(CelPolicyValidationResult compile_result,
557593
CompilePolicy(*compiler, *policy));
558594

559-
std::string tests_content;
560-
CEL_RETURN_IF_ERROR(
561-
cel::internal::GetFileContents(tests_path.string(), &tests_content));
562595
TestSuite test_suite;
563-
if (is_yaml) {
564-
CEL_ASSIGN_OR_RETURN(test_suite,
565-
cel::test::ParsePolicyTestSuiteYaml(tests_content));
566-
} else {
567-
if (!google::protobuf::TextFormat::ParseFromString(tests_content, &test_suite)) {
568-
return absl::InvalidArgumentError(
569-
absl::StrCat("Failed to parse text proto in ", tests_path.string()));
570-
}
571-
}
596+
CEL_ASSIGN_OR_RETURN(
597+
test_suite, cel::test::ParsePolicyTestSuiteYaml(sections.tests_content));
572598

599+
std::string suite_name_str(suite_name);
573600
auto runner = std::make_shared<PolicyTestSuiteRunner>(
574-
suite_name, std::move(compiler), std::move(runtime),
601+
suite_name_str, std::move(compiler), std::move(runtime),
575602
std::move(policy_source), std::move(compile_result), pool,
576603
message_factory, input_evaluator, config.GetContextType(),
577-
/*expect_compile_fail=*/absl::StrContains(suite_name, "compile_errors"));
604+
/*expect_compile_fail=*/absl::StrContains(suite_name, "compile_error"));
578605

579606
for (const auto& section : test_suite.sections()) {
580607
std::string section_name = section.name();
@@ -586,7 +613,7 @@ absl::Status RegisterTestSuite(
586613
bool skip = !ShouldRunTest(full_test_name, skip_tests);
587614

588615
testing::RegisterTest(
589-
suite_name.c_str(),
616+
suite_name_str.c_str(),
590617
absl::StrCat(section_name, "/", test_name).c_str(), nullptr,
591618
test_name.c_str(), __FILE__, __LINE__,
592619
[runner, test, full_test_name, skip]() -> CelPolicyTest* {
@@ -598,14 +625,11 @@ absl::Status RegisterTestSuite(
598625
}
599626

600627
void RegisterAllTests() {
601-
// cel::google3-end
602-
std::string testdata_example_flag = absl::GetFlag(FLAGS_testdata_example);
628+
std::vector<std::string> bundle_paths = absl::GetFlag(FLAGS_test_bundles);
603629
std::vector<std::string> skip_tests = absl::GetFlag(FLAGS_skip_tests);
604630

605-
std::string abs_testdata_example =
606-
cel::internal::ResolveRunfilesPath(testdata_example_flag);
607-
ABSL_CHECK(!abs_testdata_example.empty())
608-
<< "Could not find testdata directory: " << testdata_example_flag;
631+
ABSL_CHECK(!bundle_paths.empty())
632+
<< "No test bundles specified in --test_bundles flag.";
609633

610634
std::shared_ptr<const google::protobuf::DescriptorPool> pool =
611635
GetSharedTestingDescriptorPool();
@@ -616,35 +640,30 @@ void RegisterAllTests() {
616640
ABSL_CHECK_OK(evaluator_or.status()) << "Failed to create input evaluator";
617641
std::shared_ptr<InputEvaluator> evaluator = std::move(evaluator_or.value());
618642

619-
std::filesystem::path testdata_path(abs_testdata_example);
620-
ABSL_CHECK(std::filesystem::exists(testdata_path))
621-
<< "Testdata path does not exist: " << testdata_path;
622-
// walk up to find 'testdata' parent. A work around to portably
623-
// get the expected directory from bazel.
624-
while (!absl::EndsWith(testdata_path.string(), "testdata")) {
625-
testdata_path = testdata_path.parent_path();
626-
ABSL_CHECK(testdata_path.string().size() > sizeof("testdata"))
627-
<< "could not resolve testdata directory";
628-
}
643+
for (const std::string& bundle_path : bundle_paths) {
644+
std::string abs_path = cel::internal::ResolveRunfilesPath(bundle_path);
645+
ABSL_CHECK(!abs_path.empty())
646+
<< "Could not resolve runfile path for test bundle: " << bundle_path;
629647

630-
for (const auto& entry :
631-
std::filesystem::recursive_directory_iterator(testdata_path)) {
632-
if (!entry.is_directory()) {
633-
continue;
634-
}
635-
std::filesystem::path dir_path = entry.path();
636-
// Check if this directory has policy.yaml and tests.yaml (or
637-
// tests.textproto)
638-
if (std::filesystem::exists(dir_path / "policy.yaml") &&
639-
(std::filesystem::exists(dir_path / "tests.yaml") ||
640-
std::filesystem::exists(dir_path / "tests.textproto"))) {
641-
std::string suite_name = absl::StrReplaceAll(
642-
std::filesystem::relative(dir_path, testdata_path).string(),
643-
{{"\\", "/"}});
644-
645-
ABSL_CHECK_OK(RegisterTestSuite(dir_path, suite_name, evaluator, pool,
646-
message_factory, skip_tests));
648+
std::string bundle_content;
649+
ABSL_CHECK_OK(cel::internal::GetFileContents(abs_path, &bundle_content))
650+
<< "Failed to read bundle file: " << abs_path;
651+
652+
auto sections_or = ParseYamlBundle(bundle_content);
653+
ABSL_CHECK_OK(sections_or.status())
654+
<< "Failed to parse bundle file: " << abs_path;
655+
656+
absl::string_view filename = bundle_path;
657+
size_t last_slash = filename.find_last_of("/\\");
658+
if (last_slash != absl::string_view::npos) {
659+
filename = filename.substr(last_slash + 1);
647660
}
661+
absl::string_view suite_view = filename;
662+
absl::ConsumeSuffix(&suite_view, "_bundle.yaml");
663+
std::string suite_name = std::string(suite_view);
664+
665+
ABSL_CHECK_OK(RegisterTestSuite(suite_name, sections_or.value(), evaluator,
666+
pool, message_factory, skip_tests));
648667
}
649668
}
650669

0 commit comments

Comments
 (0)