refactoring in integration tests

[khieta]

Description of changes

• Deleted integration tests that fail to parse. With the new corpus test infrastructure, these will no longer be produced.
• Moved the cedar_test_impl.rs file (with some renaming and other additions) from cedar-spec so we can use the same trait for both integration testing & DRT.
• Refactoring in integration_testing.rs to make it easier to reuse the parsing code for the integration test file format.
• Removed the EST testing code from the integration tests. I'll add it to DRT instead.

Issue #, if available

Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

• A change "invisible" to users (e.g., documentation, changes to "internal" crates like cedar-policy-core, cedar-validator, etc.)

I confirm that this PR (choose one, and delete the other options):

• Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that cedar-spec (choose one, and delete the other options):

• Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in cedar-spec, and how you have tested that your updates are correct.)

Disclaimer

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[khieta]

Starting a draft so I don't lose my train of thought. I expect to get this ready for review tomorrow.

[khieta]

Marking this as ready for review now that 1) I have a PR up to fix the cedar-spec break (cedar-spec#212) and 2) I have a PR up to add EST tests to DRT (cedar-spec#215).

One question I'd like reviewer opinions on: as indicated by the semver check in CI, this PR is actually a breaking change since it affects the "public" APIs in cedar_policy::integration_testing and cedar_policy::cedar_test_impl. I was planning on treating this PR as non-breaking since it's only editing testing code. But technically users could be depending on our testing data types... should I mark this PR as a breaking change intended for the next major release?

[john-h-kastner-aws]

One question I'd like reviewer opinions on: as indicated by the semver check in CI, this PR is actually a breaking change since it affects the "public" APIs in cedar_policy::integration_testing and cedar_policy::cedar_test_impl. I was planning on treating this PR as non-breaking since it's only editing testing code. But technically users could be depending on our testing data types... should I mark this PR as a breaking change intended for the next major release?

It feel strange that the testing are exported from the cedar-policy crate directly. If we want to call this a breaking change, then I propose making the larger braking change, and ripping these functions out into a new crates, then that crate can follow semver without caring about cedar's version.

[khieta]

I propose making the larger breaking change, and ripping these functions out into a new crates, then that crate can follow semver without caring about cedar's version.

Yeah, I agree that it makes sense to have this testing code in a separate crate. Do you think I could get away with moving it to a separate crate before the next major version? I'd like to get this refactoring integrated sooner rather than later to prevent annoying merge issues later.

[khieta]

Update: I'll be closing this PR to avoid breaking the public interface for the integration tests. I've made an issue to move the integration testing code to its own module (#644). In the meantime, I'll move the refactoring I made in cedar_test_impl.rs back to cedar-spec#212.