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

Make result_of explain the inner matcher failure #539

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

refi64
Copy link

@refi64 refi64 commented Dec 10, 2024

Right now, explain_match() ends up calling the inner matcher's describe(). Although this works just fine for simple matchers, it falls apart for more complex ones, as it'll merely describe the matcher itself without including relevant intermediate values. For instance, this:

assert_that!(2, result_of!(|n| n, property!(i32.ilog2(), eq(10))));

results in:

Actual: 2,
  which, results into 2
    by applying |n| n,
      has property `ilog2()`, which isn't equal to 10

which omits the actual result of invoking the property.

By having it invoke explain_match(), we instead get the complete explanation:

Actual: 2,
  which, results into 2
    by applying |n| n,
      whose property `ilog2()` is `1`, which isn't equal to 10

Right now, `explain_match()` ends up calling the inner matcher's
`describe()`. Although this works just fine for simple matchers, it
falls apart for more complex ones, as it'll merely describe the matcher
itself without including relevant intermediate values. For instance,
this:

```rust
assert_that!(2, result_of!(|n| n, property!(i32.ilog2(), eq(10))));
```

results in:

```
Actual: 2,
  which, results into 2
    by applying |n| n,
      has property `ilog2()`, which isn't equal to 10
```

which omits the actual result of invoking the property.

By having it invoke `explain_match()`, we instead get the complete
explanation:

```
Actual: 2,
  which, results into 2
    by applying |n| n,
      whose property `ilog2()` is `1`, which isn't equal to 10
```
Copy link

google-cla bot commented Dec 10, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@gribozavr
Copy link
Collaborator

@refi64 Assigning back to you to resolve the CLA issue.

Description::new()
.text(format!("which, results into {actual_result:?}",))
.nested(self.describe(self.matches(actual)))
Description::new().text(format!("which, results into {actual_result:?}")).nested(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a test that demonstrates the new behavior?

@gribozavr gribozavr assigned refi64 and gribozavr and unassigned gribozavr Feb 17, 2025
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