Skip to content
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.

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

Add test_suite and test_config under conformance #441

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aakash070
Copy link

Adding test_suite and test_config protos under conformance package. These protos define:

  • contract to be used by the CEL test runner for setting up the CEL env
  • structure for test suite

@TristonianJones
Copy link
Collaborator

/gcbrun


// Representation of a CEL Environment, defining what features and extensions
// are available for conformance testing.
message Environment {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update the Environment definition to be closer to what we support as a YAML serialization format for cel-go? e.g.
https://github.com/google/cel-go/blob/69febe24b1f09fb654923ee5cfae6cb8f0ddeb26/common/env/testdata/extended_env.yaml

option java_package = "cel.dev.expr.conformance";

// The format of test suite for CEL tests.
message TestSuite {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably look at proto/cel/expr/conformance/test as the place to include these files. Also, it should be clear how this aligns with or supercedes the simple.proto in the test directory

import "cel/expr/value.proto";

option cc_enable_arenas = true;
option go_package = "cel.dev/expr/conformance";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's update the package to cel.dev/expr/conformance/test and bump the files here down a level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants