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

feat(assertions): improve printing of match failures #23453

Merged
merged 9 commits into from
Dec 28, 2022

Conversation

rix0rrr
Copy link
Contributor

@rix0rrr rix0rrr commented Dec 23, 2022

Debugging match failures with the assertions library used to be a bit frustrating. The library used to guess the single closest matching resource and print it fully, with a list of match failures underneath.

This has two problems:

  • Matching the failures up to the data structure requires a lot of scanning back and forth between different screen lines.
  • The library could incorrectly guess at the closest match, giving you useless information.

This change is a first (and by no means the last) stab at improving the situation:

  • Mismatch errors are printed in-line with the data structure, making it very clear what they are referencing (removing the need for scanning).
  • Fields in complex data structures that aren't involved in the mismatch at all are collapsed. For example, a complex subobject that matches be printed as { ... } so it doesn't take up too much vertical space.
  • Because we now have a lot of vertical space left, we can print the N closest matches, lowering the chance that we guess wrong and leave you without information to correct the test.

The last point can probably be improved more in the future, by coming up with a more accurate metric for "closest match" than "failure count", but this will do for now.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Debugging match failures with the `assertions` library used to be a bit
frustrating. The library used to guess the single closest matching
resource and print it fully, with a list of match failures underneath.

This has two problems:

- Matching the failures up to the data structure requires a lot of
  scanning back and forth between different screen lines.
- The library could incorrectly guess at the closest match, giving
  you useless information.

This change is a first (and by no means the last) stab at improving
the situation:

- Mismatch errors are printed in-line with the data structure, making
  it very clear what they are referencing (removing the need for
  scanning).
- Fields in complex data structures that aren't involved
  in the mismatch at all are collapsed. For example, a complex
  subobject that matches be printed as `{ ... }` so it doesn't take
  up too much vertical space.
- Because we now have a lot of vertical space left, we can print
  the N closest matches, lowering the chance that we guess wrong
  and leave you without information to correct the test.

The last point can probably be improved more in the future, by
coming up with a more accurate metric for "closest match" than
"failure count", but this will do for now.
@rix0rrr rix0rrr requested a review from a team December 23, 2022 11:57
@rix0rrr rix0rrr self-assigned this Dec 23, 2022
@gitpod-io
Copy link

gitpod-io bot commented Dec 23, 2022

@github-actions github-actions bot added the p2 label Dec 23, 2022
@aws-cdk-automation aws-cdk-automation requested a review from a team December 23, 2022 11:57
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Dec 23, 2022
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

@rix0rrr rix0rrr added pr-linter/exempt-readme The PR linter will not require README changes pr-linter/exempt-integ-test The PR linter will not require integ test changes labels Dec 23, 2022
@aws-cdk-automation aws-cdk-automation dismissed their stale review December 23, 2022 12:03

✅ Updated pull request passes all PRLinter validations. Dissmissing previous PRLinter review.

@mergify
Copy link
Contributor

mergify bot commented Dec 28, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 29cfc5c
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 2676386 into main Dec 28, 2022
@mergify mergify bot deleted the huijbers/mismatch-printing branch December 28, 2022 11:42
@mergify
Copy link
Contributor

mergify bot commented Dec 28, 2022

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Jan 20, 2023
Debugging match failures with the `assertions` library used to be a bit frustrating. The library used to guess the single closest matching resource and print it fully, with a list of match failures underneath.

This has two problems:

- Matching the failures up to the data structure requires a lot of scanning back and forth between different screen lines.
- The library could incorrectly guess at the closest match, giving you useless information.

This change is a first (and by no means the last) stab at improving the situation:

- Mismatch errors are printed in-line with the data structure, making it very clear what they are referencing (removing the need for scanning).
- Fields in complex data structures that aren't involved in the mismatch at all are collapsed. For example, a complex subobject that matches be printed as `{ ... }` so it doesn't take up too much vertical space.
- Because we now have a lot of vertical space left, we can print the N closest matches, lowering the chance that we guess wrong and leave you without information to correct the test.

The last point can probably be improved more in the future, by coming up with a more accurate metric for "closest match" than "failure count", but this will do for now.

----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
brennanho pushed a commit to brennanho/aws-cdk that referenced this pull request Feb 22, 2023
Debugging match failures with the `assertions` library used to be a bit frustrating. The library used to guess the single closest matching resource and print it fully, with a list of match failures underneath.

This has two problems:

- Matching the failures up to the data structure requires a lot of scanning back and forth between different screen lines.
- The library could incorrectly guess at the closest match, giving you useless information.

This change is a first (and by no means the last) stab at improving the situation:

- Mismatch errors are printed in-line with the data structure, making it very clear what they are referencing (removing the need for scanning).
- Fields in complex data structures that aren't involved in the mismatch at all are collapsed. For example, a complex subobject that matches be printed as `{ ... }` so it doesn't take up too much vertical space.
- Because we now have a lot of vertical space left, we can print the N closest matches, lowering the chance that we guess wrong and leave you without information to correct the test.

The last point can probably be improved more in the future, by coming up with a more accurate metric for "closest match" than "failure count", but this will do for now.

----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes pr-linter/exempt-readme The PR linter will not require README changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants