Skip to content

Commit

Permalink
Fix getIndexOf fallback, general assert module cleanup, refs #27
Browse files Browse the repository at this point in the history
  • Loading branch information
bitpshr authored and csnover committed May 20, 2013
1 parent c64374c commit 10a49df
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 41 deletions.
63 changes: 24 additions & 39 deletions assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ define([
'dojo/_base/array',
'dojo/json'
], function (lang, arrayUtil, JSON) {
var sliceArray = Array.prototype.slice,
objProto = Object.prototype,
var objProto = Object.prototype,
sliceArray = Array.prototype.slice,
objectToString = objProto.toString,
getObjectKeys = function (obj) {
var keys = [];
for (var k in obj) {
Expand All @@ -31,16 +32,19 @@ define([

return keys;
},
getOwnKeys = Object.keys || function (obj) {
var keys = [];
for (var k in obj) {
if (Object.prototype.hasOwnProperty.call(obj, k)) {
keys.push(k);
getIndexOf = function (haystack, needle) {
if (haystack.indexOf) {
return haystack.indexOf(needle);
}

for (var i = 0; i < haystack.length; ++i) {
if (i in haystack && haystack[i] === needle) {
return i;
}
}
return keys;

return -1;
},
objectToString = Object.prototype.toString,
inspect = (function () {
/**
* Gets the name of a function, in a cross-browser way.
Expand Down Expand Up @@ -287,18 +291,18 @@ define([
var name,
str;

if (arrayUtil.indexOf(visibleKeys, key) < 0) {
if (getIndexOf(visibleKeys, key) < 0) {
name = '[' + key + ']';
}

if (!str) {
if (arrayUtil.indexOf(ctx.seen, value[key]) < 0) {
if (getIndexOf(ctx.seen, value[key]) < 0) {
if (recurseTimes === null) {
str = formatValue(ctx, value[key], null);
} else {
str = formatValue(ctx, value[key], recurseTimes - 1);
}
if (arrayUtil.indexOf(str, '\n') > -1) {
if (getIndexOf(str, '\n') > -1) {

This comment has been minimized.

Copy link
@jdalton

jdalton May 20, 2013

getIndexOf on a string will run into issues in IE < 8 because of issues with str[index] access.
(side note there is an issue in IE8 when trying the same thing on a string object) /@bitpshr @csnover

The arrayUtil.indexOf method has a fork for string values, though I think str.indexOf(...) should do.

This comment has been minimized.

Copy link
@jdalton

jdalton May 20, 2013

Related to #27.

This comment has been minimized.

Copy link
@bitpshr

bitpshr May 20, 2013

Author Contributor

The getIndexOf fallback uses haystack.indexOf if available. String#indexOf exists in IE < 8, so the fallback shouldn't run into any issues. That being said, this particular case probably shouldn't ever have been switched to use the fallback (or arrayUtil.indexOf) to begin with.

This comment has been minimized.

Copy link
@jdalton

jdalton May 21, 2013

Word \o/!

if (array) {
str = arrayUtil.map(str.split('\n'), function (line) {
return ' ' + line;
Expand Down Expand Up @@ -437,7 +441,7 @@ define([
return str;
}
else if (type === '[object Object]') {
var keys = getOwnKeys(obj),
var keys = getObjectKeys(obj),
kstr = keys.length > 2
? (keys.splice(0, 2).join(', ') + ', ...') : (keys.join(', '));
str = '{ Object (' + kstr + ') }';
Expand Down Expand Up @@ -624,11 +628,11 @@ define([
return actual == expected;
}

else if (arrayUtil.indexOf(circularA, actual) !== -1 && arrayUtil.indexOf(circularA, actual) === arrayUtil.indexOf(circularB, expected)) {
else if (getIndexOf(circularA, actual) !== -1 && getIndexOf(circularA, actual) === getIndexOf(circularB, expected)) {
return true;
}

else if (arrayUtil.indexOf(circularA, actual) !== -1 || arrayUtil.indexOf(circularB, expected) !== -1) {
else if (getIndexOf(circularA, actual) !== -1 || getIndexOf(circularB, expected) !== -1) {
return false;
}

Expand Down Expand Up @@ -737,31 +741,12 @@ define([
};
})();

(function () {
/**
* TODO: Not necessary with es5-shim.
*/
function getIndexOf(haystack, needle) {
if (haystack.indexOf) {
return haystack.indexOf(needle);
}

for (var i = 0; i < haystack.length; ++i) {
if (haystack[i] === needle) {
return i;
}
}

return -1;
assert.include = function (haystack, needle, message) {
if (getIndexOf(haystack, needle) === -1) {
fail('', needle, message ||
('expected ' + formatValue(haystack) + ' to contain ' + formatValue(needle)), 'include', assert.include);
}

assert.include = function (haystack, needle, message) {
if (getIndexOf(haystack, needle) === -1) {
fail('', needle, message ||
('expected ' + formatValue(haystack) + ' to contain ' + formatValue(needle)), 'include', assert.include);
}
};
})();
};

assert.match = function (value, regexp, message) {
if (!regexp.test(value)) {
Expand Down
23 changes: 21 additions & 2 deletions tests/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ define([
var err = function (fn, msg) {
try {
fn();
} catch (err) {
}
catch (err) {
if ('string' === typeof msg) {
assert.equal(err.message, msg, 'Errors should be equal');
} else {
Expand All @@ -34,7 +35,19 @@ define([
throw new assert.AssertionError({ message: 'Expected an error' });
};


/*
* Executes a function; an assertion is thrown if this executed function doesn't
* throw its own assertion.
*/
var shouldThrow = function (fn, msg) {
try {
fn();
}
catch (err) {
return;
}
throw new assert.AssertionError({ message: msg || 'Expected an error' });
};

tdd.suite('assert', function () {
tdd.test('assert', function () {
Expand Down Expand Up @@ -555,6 +568,12 @@ define([

tdd.test('legacy edge cases', function () {
assert.notDeepEqual({valueOf: 1}, {}, 'own properties that shadow non-enumerable prototype properties should not be skipped');

shouldThrow(function () {
var arr = [1, 2, 3];
delete arr[2];
assert.include(arr, undefined);
}, 'Array#indexOf should skip holes in arrays');
});
});
});

0 comments on commit 10a49df

Please # to comment.