Skip to content

Conversation

@klochek
Copy link
Contributor

@klochek klochek commented Jan 28, 2026

Instead of mapping "some results" -> true. This makes for a much easier experience for users. Also, re-jigger some comparison logic, especially for jsonpath tests, since we'll need to coerce based on what we get back.

Instead of mapping "some results" -> true.  This makes for a much easier experience for users.
Also, re-jigger some comparison logic, especially for jsonpath tests, since we'll need to coerce based on what we get back.
@klochek klochek requested a review from a team as a code owner January 28, 2026 15:51
@linear
Copy link

linear bot commented Jan 28, 2026

Comment on lines +213 to +226
Comparison::LessThan => match self {
JSONPathOperand::None => false,
JSONPathOperand::Literal { value } => cmp_lt_json(v, value, gas)?,

JSONPathOperand::Glob { .. } => false,
},
Comparison::GreaterThan => match self {
JSONPathOperand::None => false,
JSONPathOperand::Literal { value } => cmp_gt_json(v, value, gas)?,

JSONPathOperand::Glob { .. } => false,
},
Comparison::Equal => cmp_eq_jsonpath(v, self, gas)?,
Comparison::NotEqual => !cmp_eq_jsonpath(v, self, gas)?,
Copy link
Member

Choose a reason for hiding this comment

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

Do all of these comparisons have tests that validate them? I didn't see it in the new tests, but wasn't sure if they already existed

super::JSONPathOperand::Literal { value } => JSONPathOperand::Literal {
value: value.clone(),
},
super::JSONPathOperand::Glob { pattern: value } => {
Copy link
Member

Choose a reason for hiding this comment

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

Same question here about whether this has tests

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