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

Optionally hide whitespace changes in diff #8626

Closed
yassinecc opened this issue Jul 1, 2019 · 5 comments · Fixed by #9203
Closed

Optionally hide whitespace changes in diff #8626

yassinecc opened this issue Jul 1, 2019 · 5 comments · Fixed by #9203

Comments

@yassinecc
Copy link

🚀 Feature Proposal

Add a CLI option to ignore whitespace changes when displaying diffs.

Motivation

When using Jest snapshots, Jest reports in case of failure the diff between expected and received outputs. In some cases, the snapshot is so large that adding a top level node in the snapshot results in a huge, unreadable diff (over say 10000 lines), while the change in itself is minimal.

Example

Would be nice to have an option like jest --ignore-whitespace

Pitch

Why does this feature belong in the Jest core platform?

Common feature proposals that do not typically make it to core:

  • New matchers (see jest-extended)
  • Changes to the default reporter (use custom reporters instead)
  • Changes to node/jsdom test environments (use custom environments instead)
@thymikee
Copy link
Collaborator

thymikee commented Jul 1, 2019

cc @pedrottimark

@pedrottimark
Copy link
Contributor

@yassinecc Do you mean ignore spaces at beginning of line only?

Hard to believe that git-diff has --ignore-space-at-eol but not --ignore-space-at-bol

@yassinecc
Copy link
Author

Hello @pedrottimark, yes indeed. I see that you merged a related PR, did it address this issue? If so we can probably close this one

@pedrottimark
Copy link
Contributor

pedrottimark commented Nov 14, 2019

@yassinecc To make a long story short, you can leave the issue open.

This issue directly influenced refactoring and exporting diffLinesUnified2 function in a series of pull requests to document a general-purpose API for jest-diff package, so third-party dependents can configure it for other applications than Jest.

Ignoring format indentation has been available when deep-equality matchers like toEqual fail:

const expectedLines2 = format(expected, {...formatOptions, indent: 2}).split('\n');
const receivedLines2 = format(received, {...formatOptions, indent: 2}).split('\n');

const expectedLines0 = format(expected, {...formatOptions, indent: 0}).split('\n');
const receivedLines0 = format(received, {...formatOptions, indent: 0}).split('\n');

const difference = diffLinesUnified2(
  expectedLines2, // snapshot
  receivedLines2,
  expectedLines0, // not directly available for snapshot tests
  receivedLines0,
);

Unlike expect(received).toEqual(expected) which can format both values with and without indentation, the snapshot without indentation is not directly available.

  1. An elegant solution for inline snapshots is if prettier writes literal ECMAScript arrays and objects and also JSX notation for elements (instead of formatted serialization) so Jest could format both values like an ordinary matcher.

    It seems like that was the original goal, but because I was on a break from Jest at that time, I don’t know how much work to change them.

  2. A pragmatic solution for inline and external snapshots is for a heuristic algorithm to un-indent the snapshot to compute expectedLines0 when an assertion fails.

    I had attempted that solution last year, and during recent weeks I rewrote the algorithm from scratch. I got stuck on the same edge case as before: if multi-line strings have indentation as content (for example, code example in pre element) then it is hard to distinguish content indentation from format indentation.

    I will take a break from it for a while, and then try again with a refreshed mind:

    • can the heuristic detect the rare edge case of multi-line string as attribute, property, or text node, and then fall back to the baseline diffLinesUnified(snapshot, receivedLines2)
    • otherwise automatically ignore indentation similar to other assertions

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants