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

Include actual/expected values in error events #2123

Open
DanTup opened this issue Oct 18, 2023 · 4 comments
Open

Include actual/expected values in error events #2123

DanTup opened this issue Oct 18, 2023 · 4 comments
Labels
type-enhancement A request for a change that isn't a bug

Comments

@DanTup
Copy link
Contributor

DanTup commented Oct 18, 2023

VS Code runs test with -r json and uses the data to populate its test runner. The API allows us to provide expected/actual values for a test failure which can be rendered as a diff:

Screenshot 2023-10-18 160512

Screenshot 2023-10-18 160529

This could be more convenient for some operations (such as copy/pasting the raw expected value), but it requires that we have these values in the JSON payloads in their raw form (not embedded in some string).

@natebosch
Copy link
Member

I think the diff view will definitely help with String values. Are there any values where the VSCode diff is a downgrade? Do we lose the "Which" string entirely?

@jakemac53 jakemac53 added the type-enhancement A request for a change that isn't a bug label Oct 18, 2023
@DanTup
Copy link
Contributor Author

DanTup commented Oct 18, 2023

Are there any values where the VSCode diff is a downgrade?

I don't know, but it's worth noting that the API only accepts strings, so whatever we have we need to stringify it and it'll get a text diff.

Do we lose the "Which" string entirely?

It shows up in the Debug Console - but it doesn't appear elsewhere. However I feel like this is a VS Code bug (because it forces us to provide a message that isn't shown - the Debug Console output is from another source). I'll file an issue about that.

Edit: Actually it is visible in the title of the peek pane, but it won't handle long values. I've filed microsoft/vscode#195920.

@natebosch
Copy link
Member

We would need to figure out some way to identify when it will be useful to do this. In many cases our "Expected:" and "Actual:" lines are too unrelated for a diff to mean anything.

For example, with matcher:

   expect(Future.value('sad'), completion(equals('happy')));

Output:

  Expected: completes to a value that 'happy'
    Actual: <Instance of 'Future<String>'>
     Which: emitted 'sad'
              which is different.
                    Expected: happy
                      Actual: sad
                              ^
                     Differ at offset 0

What I think we'd ideally want is to somehow plumb through the 'happy' and 'sad' strings, and then maybe have this full output as the extra context message (assuming the linked vscode issue is resolved). I don't know how difficult it may be to get matcher to output something useful for this pattern.

In checks the overall "Expected" and "Actual" formatting are more structured than they were in matcher, so more of the prefix before the interesting diff will be identical, but it always has some checks related diff.

    await check(Future.value('sad')).completes((r) => r.equals('happy'));

Output:

  Expected: a Future<String> that:
    completes to a value that:
      equals 'happy'
  Actual: a Future<String> that:
    completes to a value that:
    Actual: 'sad'
    Which: differs at offset 0:
    happy
    sad
    ^

In the the checks implementation we also have a string representation of the specific value that caused the inner failure when we throw the exception, so we can get around this Future case when the ultimately failing expectation is an equals comparison on String, but other cases will still have an "expected" description that won't necessarily diff nicely with the "actual".

Maybe it's only the string equality case where the extra diff is useful? Deep collection equality could produce a useful diff, but only if we override the tostring output and format collections without any eliding any elements.

@DanTup
Copy link
Contributor Author

DanTup commented Oct 23, 2023

Maybe it's only the string equality case where the extra diff is useful?

In some of the examples above it says "which differs at offset x". Perhaps all cases that currently build a string like that would be useful (even if they're not exactly a string equality at the top)? (and maybe there are some related ones like "has extra trailing characters"?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

3 participants