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

A way to display *why* a predicate failed #60

Merged
merged 7 commits into from
Jul 28, 2018
Merged

Conversation

epage
Copy link
Contributor

@epage epage commented Jul 21, 2018

Output of case_tree example:

var is 5
((! var != 5) && var >= 5)
├── (! var != 5)
|   └── var != 5
└── var >= 5

An extension trait is added to make a Displayable struct that will print out the above tree.

  • This is built off of find_case which will find the Case that prove whether the Predicate succeeds or fails.
  • To reduce overhead, Case only owns by-Products of the Predicate evaluation, parameters are made available as references on the PredicateReflection trait
    • This had the benefit of simplifying the Case API because Parameter and Product have different ownership requirements.
  • PredicateReflection is also use to homogenize the predicate tree
    • Because of predicate adapters, you could have a tree that includes Predicate<[u8]>, Predicate<str>, etc.

Once this is in, to reduce API breaks from impacting the assert-ecosystem, #30 will be implemented and this repo will be turned into a workspace for

  • predicate-core: core.rs, reflection.rs
  • predicate-tree: tree.rs
  • predicates
    • All existing predicates
    • Re-export predicate-core

aka the only change to most end-users will be predicate-tree being removed from the API while the assert-ecosystem will use predicate_core::Predicate in its public API.

This supersedes #39 due to having an API that we are more comfortable moving to 1.0

  • Unlike WIP: Pretty error printer for predicates. #39, we do not include item in the Display, allowing us to avoid requiring Item: Debug. Instead we just rely on every Predicate implementing Display, usually doing so by having an Item: Debug requirement but clients are free to bypass that by creating their own Predicate.

BREAKING CHANGES

  • Predicates must also implement PredicateReflection. Provided methods make this a impl PredicateReflection for X {}

@epage
Copy link
Contributor Author

epage commented Jul 21, 2018

@nastevens my main concern with having this reviewed is regarding vision

Luckily, I was able to avoid trait Predicate where Item: Debug but I do have Predicate : PredicateReflection : Display. I tried to keep the requirements for implementing a Predicate to a minimum, allowing people to write more code to opt-in to richer reporting features.

@epage
Copy link
Contributor Author

epage commented Jul 21, 2018

@nastevens btw for time table, the goal is to have this PR in, #30 in, and assert_fs / assert_cmd switched over to these changes by the end of the month in preparation for the next 2018 Edition preview.

Predicates can now report parameter details that don't show up in
`Display` as well as what children they have.  The expectation is that
this will be used by the solution for assert-rs#7.

BREAKING CHANGE: `Predicate`s must also implement
`reflection::PredicateReflection`.
This also saw an audit of predicates to ensure their `Display` would
most likely remain as one line of text.  Anything in a `Display` that
seemed likely to overflow was moved to a `Parameter`.
This is the last of the foundation for resolving assert-rs#7.
A non-default implementation is provided for all predicates that can add
additional information than what `eval` / `Display` provide.

Exceptions
- `eq_file`: I want to re-work this predicate
- `BoxPredicate`: The where clause is causing problems

This is the information needed for resolving assert-rs#7.  Now, rendering is the
only thing left.
Apparently, a provided function can't downcast `&self` but I can if I
spread the implementation around.

Overall, trying to meet the goal of making it simple to write a
Predicate.  Anyone besides us will need to copy `default_find_case` if
they want the reference.  Not sure what would be a good way to make that
good enough for being stable.
Finnaly, Fixes assert-rs#7.

Inspired by the work in assert-rs#39.

Created softprops/treeline#3 for trying to find ways to make this more
efficient.
@epage epage merged commit cc1dab0 into assert-rs:master Jul 28, 2018
@epage epage deleted the tree branch July 28, 2018 23:32
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