Skip to content

Implement Request.Equal() method#112

Merged
patjakdev merged 3 commits intocedar-policy:mainfrom
gregory-nisbet:2025-10-07-add-request-equal
Oct 28, 2025
Merged

Implement Request.Equal() method#112
patjakdev merged 3 commits intocedar-policy:mainfrom
gregory-nisbet:2025-10-07-add-request-equal

Conversation

@gregory-nisbet
Copy link
Copy Markdown
Contributor

Implement Request.Equal(), include a basic check that it's working correctly, and check that cmp.Diff can consume two requests and compare them.

Issue #, if available:

#107

Description of changes:

Adds Request.Equal and some tests.

for j, s := range testcases[:i] {
expected := i == j
actual := r.Equal(s)
if actual != s.Equal(r) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It might be nice if instead of running the testcases against the testcases, to have specific left/right and expected equality values. (Then you could check the equality values using testutil.Equals)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

e.g. struct {
  left Request
  right Request
  want bool
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Copy Markdown
Contributor

@philhassey philhassey left a comment

Choose a reason for hiding this comment

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

Looks good, just a minor improvement to the test structure would be nice.

@philhassey
Copy link
Copy Markdown
Contributor

Sorry for the delay in feedback here, notifications weren't set up right.

@philhassey
Copy link
Copy Markdown
Contributor

@gregory-nisbet Also there's some lint complaints:

image

@gregory-nisbet gregory-nisbet force-pushed the 2025-10-07-add-request-equal branch from 9abb4f9 to f623698 Compare October 25, 2025 17:45
Implement Request.Equal(), include a basic check that it's working
correctly, and check that cmp.Diff can consume two requests and
compare them.

Signed-off-by: Greg NISBET <gregory.nisbet@gmail.com>
Signed-off-by: Greg NISBET <gregory.nisbet@gmail.com>
Signed-off-by: Greg NISBET <gregory.nisbet@gmail.com>
@gregory-nisbet gregory-nisbet force-pushed the 2025-10-07-add-request-equal branch from 9b256c2 to 73efbdc Compare October 25, 2025 20:19
@patjakdev patjakdev requested a review from philhassey October 28, 2025 14:56
@patjakdev patjakdev dismissed philhassey’s stale review October 28, 2025 14:57

Feedback addressed

@patjakdev patjakdev merged commit 15728b3 into cedar-policy:main Oct 28, 2025
2 checks passed
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.

3 participants