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

_.escape test suit #2794

Closed
lucky06688 opened this issue Jan 9, 2019 · 1 comment
Closed

_.escape test suit #2794

lucky06688 opened this issue Jan 9, 2019 · 1 comment

Comments

@lucky06688
Copy link

// handles multiple escape characters at once
var joiner = ' other stuff ';
var allEscaped = escapeCharacters.join(joiner);
allEscaped += allEscaped;
**assert.ok(_.every(escapeCharacters, function(escapeChar) {
  return allEscaped.indexOf(escapeChar) !== -1;
}), 'handles multiple characters');**
assert.ok(allEscaped.indexOf(joiner) >= 0, 'can escape multiple escape characters at the same time');

I can't understand this test in which the 'allEscaped' is not escaped. https://github.com/jashkenas/underscore/blob/v1.9.0/test/utility.js#L142-L14

@jgonggrijp jgonggrijp added the bug label Apr 7, 2020
@jgonggrijp
Copy link
Collaborator

@lucky06688 Thanks for reporting. It appears indeed that allEscaped is never used to actually test the behaviour of _.escape and _.unescape. This is still an issue on master:

underscore/test/utility.js

Lines 162 to 169 in 2a93247

// handles multiple escape characters at once
var joiner = ' other stuff ';
var allEscaped = escapeCharacters.join(joiner);
allEscaped += allEscaped;
assert.ok(_.every(escapeCharacters, function(escapeChar) {
return allEscaped.indexOf(escapeChar) !== -1;
}), 'handles multiple characters');
assert.ok(allEscaped.indexOf(joiner) >= 0, 'can escape multiple escape characters at the same time');

It appears this has always been the case since these lines were first added in 9c1c3ea. The author probably forgot.

jgonggrijp added a commit to jgonggrijp/underscore that referenced this issue Aug 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants