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

An eval that tells me why the predicate failed #7

Closed
epage opened this issue Mar 28, 2018 · 4 comments
Closed

An eval that tells me why the predicate failed #7

epage opened this issue Mar 28, 2018 · 4 comments
Assignees
Labels
breaking-change enhancement Improve the expected
Milestone

Comments

@epage
Copy link
Contributor

epage commented Mar 28, 2018

In using this with assertions, like assert_cli, it'd be a very big help for debuggability to tell people why the assertion failed.

Requirements

Some possible requirements include (yes, some will be contradictory)

  • low overhead in the success case
  • be clear about what all the values shown mean
    • e.g. is_close has several params. If do we inline substitution, it might not be clear
  • don't overwhelm the user
  • show me all of the failures because the first might not be the root cause (particularly when doing dir-diffing)
  • show me all of the successes because a later failure might be rooted in the wrong thing succeeding
  • Make things clear in the formatting
  • Programmatically report the result so assert_cmd and friends can decide whether to panic (return error, etc)

Example of odd cases to watch for

  • is_close taking several parameters
  • from_utf8, trim, a potential normalize_newlines, from_file_path transform the data being passed down
    • Some of these also changes item's type
  • name shifts the child predicate into an equivalent of a named field
  • not flips what case we are looking for

Open Questions

  • Is this feature important enough and Debug implementations common enough that we can justify requiring Predicate<Item: Debug>?
    • atm we're leaning towards this being acceptable

API Design Options

cases_iter(item, yes: bool) -> Iterator

This returns an iterator of examples of cases for why yes. Each case in the iterator can show the full expression tree and what each part evaluates to.

The yes is needed to flip what we are looking for when evaluating not.

tree_eval(item) -> (bool, tree)

Returns the result of eval plus a renderable tree using the treeline crate.

Requires the following Predicate methods

  • stringify: like Display but substitutes the Item in
  • make_tree: creates the renderabvle tree
  • tree_eval: the actual function, a default trait implementation exists

See #39

table_eval(item) -> (bool, table)

Like tree_eval but render a table using term-table

See #39

prove(&'a self, item, yes:bool) -> Option<Tree<'a>>

Creates a Tree of the predicates and their intermediate values using references, owned values, and Cow. Different render implementations can be written against this, whether tree, table, or something else.

Only return those parts for support yes.

If None is returned, then nothing supports yes.

@epage epage added the enhancement Improve the expected label Mar 28, 2018
epage added a commit to epage/predicates-rs that referenced this issue Apr 1, 2018
This will be more helpful when assert-rs#7 is implemented.

Fixes assert-rs#9
epage added a commit to epage/predicates-rs that referenced this issue Apr 2, 2018
This will be more helpful when assert-rs#7 is implemented.

Fixes assert-rs#9
epage added a commit to epage/predicates-rs that referenced this issue Apr 2, 2018
This will be more helpful when assert-rs#7 is implemented.

Fixes assert-rs#9
epage added a commit to epage/predicates-rs that referenced this issue Apr 4, 2018
This will be more helpful when assert-rs#7 is implemented.

Fixes assert-rs#9
@epage

This comment has been minimized.

@epage
Copy link
Contributor Author

epage commented Apr 20, 2018

Given:

let between_5_and_10 = predicate::ge(5).and(predicate::le(10));
assert_eq!(true, between_5_and_10.eval(&7));
assert_eq!(false, between_5_and_10.eval(&11));
assert_eq!(false, between_5_and_10.eval(&4));

ideally the output would be

item > 5 && item < 10
with
    item: 7

And because of not, we need to always build this up because a predicate that might be "fine" can be turned into the "failure case".

So API requirements

  • Build up expression and values separately
  • Allow user to pass in the variable name to meet the current application
  • Allow predicate transforms to define new variables (taking a path predicate and turning it into a str predicate by reading its contents)
  • Might get recursive if we allow naming sub-expressions, which would be ideal for files

@epage epage added this to the 0.4 milestone Apr 27, 2018
@epage

This comment has been minimized.

epage added a commit to epage/predicates-rs that referenced this issue May 15, 2018
This is a step towards assert-rs#7.  We can now display the expression under
test.

Next steps:
- Ellipsis the strings being reported from this change.
- Ability to enumerate failure cases.

I'm mixed about building this enumeration functionality directly into
`Predcate` or if we should instead provide an expanded trait that is
only implemented when `Predicate` is `Display` and `T` is `Debug`.  For
now, I'm going with the simplest implementation, to assume every
predicate will want this functionality.

BREAKING CHANGES:
- `Display` is now required to implement `Predicate`.
- Generic traits (taking `T`) now require `T` to be `Debug`.
epage added a commit to epage/predicates-rs that referenced this issue May 15, 2018
This can be important for
- Clarifying the intent of a predicate, especially when looking at test
  results
- Shortening `Predicate::display`s that are too big

This is another step towards assert-rs#7.
epage added a commit that referenced this issue May 15, 2018
Implement `Display` support in lead up to #7
@epage
Copy link
Contributor Author

epage commented Jun 16, 2018

A great example of a failure message is from pytest

___________________________________________________________ test_session_container ___________________________________________________________
                                                                                                                                              
    def test_session_container():                                                                                                             
>       helper(pathlib.Path(__file__), "bye")                                                                                                 
                                                                                                                                              
tests/test_convert.py:24:                                                                                                                     
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
                                                                                                                                              
one = PosixPath('/home/epage/git/nixnet-python/tests/test_convert.py'), two = 'bye'                                                           
                                                                                                                                              
    def helper(one, two):                                                                                                                     
        expected_one = 5                                                                                                                      
        expected_two = "hello"                                                                                                                
>       assert expected_one == one.parent.parent.with_suffix(".foo") and expected_two == two                                                  
E       AssertionError: assert (5 == PosixPath('/home/epage/git/nixnet-python.foo'))                                                          
E        +  where PosixPath('/home/epage/git/nixnet-python.foo') = <bound method PurePath.with_suffix of PosixPath('/home/epage/git/nixnet-python')>('.foo')                                                                                                                                
E        +    where <bound method PurePath.with_suffix of PosixPath('/home/epage/git/nixnet-python')> = PosixPath('/home/epage/git/nixnet-python').with_suffix                                                                                                                              
E        +      where PosixPath('/home/epage/git/nixnet-python') = PosixPath('/home/epage/git/nixnet-python/tests').parent                    
E        +        where PosixPath('/home/epage/git/nixnet-python/tests') = PosixPath('/home/epage/git/nixnet-python/tests/test_convert.py').parent                                                                                                                                          
                                                                                                                                              
tests/test_convert.py:20: AssertionError                                                                                                      

epage added a commit to epage/predicates-rs that referenced this issue Jul 6, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 6, 2018
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.

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.

BREAKING CHANGE: `Predicate`s must also implement
`reflection::PredicateReflection`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 11, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 13, 2018
This is the last of the foundation for resolving assert-rs#7.
@epage epage self-assigned this Jul 17, 2018
epage added a commit to epage/predicates-rs that referenced this issue Jul 19, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 19, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 20, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 20, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 20, 2018
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.
epage added a commit to epage/predicates-rs that referenced this issue Jul 20, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 20, 2018
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.
epage added a commit to epage/predicates-rs that referenced this issue Jul 21, 2018
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 added a commit to epage/predicates-rs that referenced this issue Jul 21, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 21, 2018
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.
epage added a commit to epage/predicates-rs that referenced this issue Jul 21, 2018
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 added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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 added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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 added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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`.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
This is the last of the foundation for resolving assert-rs#7.
epage added a commit to epage/predicates-rs that referenced this issue Jul 28, 2018
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.
@epage epage closed this as completed in 67085fc Jul 28, 2018
epage added a commit that referenced this issue Oct 6, 2023
README.md list indentation and no bare URLs, as per Markdown Lint VS Code extension
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change enhancement Improve the expected
Projects
None yet
Development

No branches or pull requests

1 participant