Skip to content

Add corpus coverage cases and validation updates#142

Closed
philhassey wants to merge 1 commit intocedar-policy:mainfrom
strongdm:corpus-2026-03-18
Closed

Add corpus coverage cases and validation updates#142
philhassey wants to merge 1 commit intocedar-policy:mainfrom
strongdm:corpus-2026-03-18

Conversation

@philhassey
Copy link
Contributor

@philhassey philhassey commented Mar 18, 2026

Summary

This PR refreshes corpus coverage and aligns cedar-go validation/typechecking behavior more closely with Rust Cedar.

What changed

  • Refreshed corpus artifacts:
    • corpus-tests.tar.gz
    • corpus-tests-json-schemas.tar.gz
    • corpus-tests-validation.tar.gz
  • Added new corpus coverage scenarios and expected outputs:
    • coverage_full
    • compare_ifset_coverage
    • type_order_coverage
    • policy_env_error_merge
  • Updated Makefile corpus extraction/coverage rules used by corpus regeneration and CI checks.
  • Validator/typechecker updates in x/exp/schema/validate:
    • Added stable merge identity support for diagnostics (diag_error.go) and improved cross-environment error deduping.
    • Updated action in [] behavior to match Rust semantics (strict vs permissive handling).
    • Aligned behavior for if-then-else, ||, equality folding (including action rewrite), is ... in, set incompatibility reporting, and strict extension-constructor literal checks.
    • Added environment-aware merge keying for unsafe tag access diagnostics.
  • Cedar type rendering/order updates in cedar_type.go:
    • Structural comparator for deterministic Rust-like type ordering in incompatibility errors.
    • Updated rendered names/record optional attribute formatting to match Rust output.
  • Rust string escaping updates in internal/rust:
    • Split escaping into debug/display modes.
    • Use display-style escaping for extension argument parse errors.
  • Updated tests and validation snapshots to reflect the new behavior.

Validation

  • go test ./...
  • go vet ./...

Signed-off-by: Phil Hassey <phil@strongdm.com>
Copy link
Collaborator

@patjakdev patjakdev left a comment

Choose a reason for hiding this comment

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

There are a lot of changes I don't fully understand. But I have faith in the test methodology.

Comment on lines -39 to -48
func (typeNever) isCedarType() { _ = "hack for code coverage" }
func (typeTrue) isCedarType() { _ = "hack for code coverage" }
func (typeFalse) isCedarType() { _ = "hack for code coverage" }
func (typeBool) isCedarType() { _ = "hack for code coverage" }
func (typeLong) isCedarType() { _ = "hack for code coverage" }
func (typeString) isCedarType() { _ = "hack for code coverage" }
func (typeSet) isCedarType() { _ = "hack for code coverage" }
func (typeRecord) isCedarType() { _ = "hack for code coverage" }
func (typeEntity) isCedarType() { _ = "hack for code coverage" }
func (typeExtension) isCedarType() { _ = "hack for code coverage" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmm, why did Codex change this?

// Expression type checking
allEnvs := v.generateRequestEnvs()
envs := v.filterEnvsForPolicy(allEnvs, principalTypes, resourceTypes, actionUIDs)
actionScopeEmptySet := false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maaan, this whole thing still doesn't make a ton of sense to me. This behavior maybe seems more correct?

go test -coverprofile=coverage.out ./...
sed -i '' '/^github.com\/cedar-policy\/cedar-go\/internal\/schema\/parser\/cedarschema.go/d' coverage.out
go tool cover -func=coverage.out | sed 's/%$$//' | awk '$$2 == "isCedarType" { next } $$2 == "Entity" && $$1 ~ /entity\.go/ { next } $$2 == "typeOfExtensionCall" { next } { if ($$3 < 100.0) { printf "Insufficient code coverage for %s\n", $$0; failed=1 } } END { exit failed }'
go tool cover -func=coverage.out | sed 's/%$$//' | awk '$$1 == "total:" { next } { if ($$3 < 100.0) { printf "Insufficient code coverage for %s\n", $$0; failed=1 } } END { exit failed }'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this need to change in the .github CI as well? I'm not sure what it's doing here.

@rm -rf /tmp/corpus-tests /tmp/corpus-tests-json-schemas
@mkdir -p /tmp/corpus-tests-json-schemas
@tar -xzf corpus-tests.tar.gz -C /tmp/
@tar -xzmf corpus-tests.tar.gz -C /tmp/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure why -m is desirable exactly.

@philhassey philhassey closed this Mar 18, 2026
@philhassey philhassey deleted the corpus-2026-03-18 branch March 18, 2026 22:00
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