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

Add some more exhaustive tests of ignoreWhitespace based on @Mingun's work #488

Merged

Conversation

ExplodingCabbage
Copy link
Collaborator

As suggested at #486 (comment), I've copied the tests @Mingun previously wrote for this feature (with minor tweaks).

Thanks for the help, @Mingun!

@ExplodingCabbage ExplodingCabbage merged commit a3e4812 into master Feb 15, 2024
@ExplodingCabbage ExplodingCabbage deleted the incompatible-newline-options-steal-minguns-tests branch February 15, 2024 17:49
const diffResult = diffTrimmedLines(
stringify([10, 20, 30]),
stringify({ data: [10, 42, 30] }));
expect(diffResult.filter(change => !change.removed).map(change => change.value).join('')).to.equal(`{
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you decide not to use convertChangesToXML as was in original? It is simpler assert the whole result rather than part of it to make test less fragile, isn't it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to make the full JSON text with all its indentation clearly visible & readable as plain text, so it's easy to confirm by eyeballing the test case that the indentation we're checking for is correct. But you're probably right that your way is better; I'll add it in as well.

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.

2 participants