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

test: use cel expression builder in response verification integration tests #244

Closed
wants to merge 4 commits into from

Conversation

nathanosdev
Copy link
Member

@nathanosdev nathanosdev commented Dec 5, 2023

Using the CEL builder in our integration tests means we have a lot less handwritten CEL expressions. Eventually I'd like to get rid of all of them except for the ones that are used to test the CEL parser and builder.

There's still a few more around, but refactoring the tests wasn't my focus. I ran into issues in these tests because of changes that are coming in another PR. The Certification object from ic-http-certification is using references now, so after integrating with that the CEL parser functions have some lifetime restrictions on their parameters that makes them difficult to work with in certain situations and it was much easier to just use the CEL builder in the tests.

@nathanosdev nathanosdev self-assigned this Dec 5, 2023
@nathanosdev nathanosdev force-pushed the nathan/use-cel-builder-integration-tests branch from 1475b61 to 6df5d3a Compare December 5, 2023 11:43
Base automatically changed from nathan/create-cel-expr-bug to main December 5, 2023 12:22
@nathanosdev nathanosdev force-pushed the nathan/use-cel-builder-integration-tests branch from 6df5d3a to 05a527f Compare December 5, 2023 14:33
…:dfinity/response-verification into nathan/use-cel-builder-integration-tests
@nathanosdev nathanosdev closed this Dec 7, 2023
@nathanosdev nathanosdev deleted the nathan/use-cel-builder-integration-tests branch December 7, 2023 10:07
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.

1 participant