chore: convert compareArray() to assert.compareArray() #3219
Conversation
aed2d60 to
b6ba2e1
Compare
478d995 to
7e4b2ad
Compare
7e4b2ad to
92dad8d
Compare
harness/compareArray.js
Outdated
| @@ -27,7 +27,7 @@ compareArray.isSameValue = function(a, b) { | |||
| }; | |||
|
|
|||
| compareArray.format = function(spreadable) { | |||
| return `[${[...spreadable].map(String).join(', ')}]`; | |||
| return `[${[].map.call(spreadable, String).join(', ')}]`; | |||
There was a problem hiding this comment.
Dunno if this will work--a "spreadable" is an iterable, right?
There was a problem hiding this comment.
there is a whole problematic over this compareArray file and this format is being only used for the assertion message.
The assertion requires the values a and b (actual and expected) to be accessed in a for loop during the assertion, and the values would be accessed again for this formatting purposes.
I believe this [].map.call is a conservative approach and I'd stick with it for discussing compareArray in a different PR.
…ual and expected more than once, unless absolutely necessary.
| // more than once unless absolutely necessary. | ||
| if (!result) { | ||
| assert(false, `Expected ${format(actual)} and ${format(expected)} to have the same contents. ${message}`); | ||
| } |
There was a problem hiding this comment.
@jugglinmike Rick and I discussed about this assertion message that is now deferred to execute only if compareArray fails. That might avoid conflicts for existing tests migrating from pure compareArray to assert.compareArray.
|
This PR mangled my previous PR #3213 (and possibly other changes!) |
|
|
||
|
|
||
| // Indirect change requested through Object.freeze | ||
|
|
||
| // Try freezing more times than there are exported properties | ||
| for (let i = 1; i < exported.length + 2; i++) { | ||
| assert.throws( | ||
| TypeError, | ||
| function () { | ||
| Object.freeze(ns); | ||
| }, | ||
| "Object.freeze: " + String(i) | ||
| ); | ||
| } | ||
|
|
||
| for (const key of exported) { | ||
| const desc = Object.getOwnPropertyDescriptor(ns, key); | ||
| assert.sameValue(desc.writable, true, String(key) + " writable"); | ||
| } | ||
|
|
||
| assert(!Object.isFrozen(ns), "namespace object not frozen"); |
No description provided.