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

Provide options for assert.strictEqual to display full strings #19106

Closed
joyeecheung opened this issue Mar 3, 2018 · 7 comments
Closed

Provide options for assert.strictEqual to display full strings #19106

joyeecheung opened this issue Mar 3, 2018 · 7 comments
Assignees
Labels
assert Issues and PRs related to the assert subsystem.

Comments

@joyeecheung
Copy link
Member

When calling assert.strictEqual() on long strings (>128 chars) they will be cropped, and if the first different character appear after the 128th character, all we have is '128 chars of common prefix... strictEqual '128 chars of common prefix... which is not really debuggable. This is very common when comparing error messages and file contents. Maybe we can provide an environment variable to toggle the output to

'long string'

strictEqual

'long string'

So one does not have to alter the code to get the difference of such long strings.

@joyeecheung joyeecheung added the assert Issues and PRs related to the assert subsystem. label Mar 3, 2018
@joyeecheung
Copy link
Member Author

cc @BridgeAR

@BridgeAR BridgeAR self-assigned this Mar 3, 2018
@BridgeAR
Copy link
Member

BridgeAR commented Mar 3, 2018

We are on the same page that it is bad if the user can not see the difference. Providing a environment variable does not seem the best way to handle this though.

I thought about adding options to assert.deepStrictEqual as well at some point but having solid defaults is still best for now out of my perspective. The output also differs depending on using the strict mode or legacy mode. The strict mode has a much better output already but it could still be improved.

Specifically about limiting the output: I suggest to make it a bit smarter by checking where the input differs and to show the string from there on. As in:

const a = `${'a'.repeat(200)}bbb${'a'.repeat(100)}bbbaaaaa`
const b = `${'a'.repeat(200)}ccc${'a'.repeat(100)}cccaaaaa`
assert.strictEqual(a, b)
// AssertionError [ERR_ASSERTION]: Input A expected to strictEqual input B:
// + expected - actual ...[n]... identical characters skipped
//
//  - '[197]...aaabbbaaa...[94]...aaabbbaaaaa'
//  + '[197]...aaacccaaa...[94]...aaacccaaaaa'
//
//  StackTrace

Input is much appreciated about making the output even clearer. I would only skip characters if there are more than 10 identical ones in a row.

@BridgeAR
Copy link
Member

BridgeAR commented Mar 3, 2018

I still believe we should limit the total output with my suggestion but we can raise that limit to e.g. 256 characters in total. Thoughts?

@BridgeAR
Copy link
Member

BridgeAR commented Mar 3, 2018

The downside of this is of course that theoretically the input could also look like this. I am still trying to figure out a way to visualize the difference between such cases. One way is to show colors but that will only work on a TTY that supports it and we can not always expect that.

@BridgeAR
Copy link
Member

BridgeAR commented Mar 3, 2018

const a = `${'a'.repeat(200)}bbb${'a'.repeat(100)}bbb[...]aaaaa`
const b = `${'a'.repeat(200)}ccc${'a'.repeat(100)}ccc[...]aaaaa`
assert.strictEqual(a, b)
// AssertionError [ERR_ASSERTION]: Input A expected to strictEqual input B:
// + expected - actual [...] identical characters skipped with the number below 
//
// - '[...]aaabbbaaa[...]aaabbb[...]aaaaa'
// + '[...]aaacccaaa[...]aaaccc[...]aaaaa'
//    ^ 197         ^ 94
// 
//  StackTrace

This might be a better alternative?

@joyeecheung
Copy link
Member Author

We can always try our best in the default but it is important to have a way to dump everything out if we are not clever enough IMO. Otherwise whenever we have edge cases that do not work well we will have to go back to fix assert before getting anything debuggable.

@BridgeAR
Copy link
Member

BridgeAR commented Mar 3, 2018

@joyeecheung so right now the default in strict mode shows the whole line while legacy mode shows a limited output. I suggest to change the output of the legacy mode to be identical with the strict mode for the functions with strict in their name (the new output is incompatible with loose comparison).

On top of that I would like to change the default for one line outputs to the one above while I am fine with showing the whole output as a default and adding a environment variable to limit the output or the other way around.

BridgeAR added a commit to BridgeAR/node that referenced this issue Aug 4, 2018
This adds the `NODE_ASSERT_FULL` environment variable to give the
user the option to opt into seeing the full error message even though
the diff is at least partially identical.

Fixes: nodejs#19106
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assert Issues and PRs related to the assert subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants