From f782971ad292b4cde1de9c35d91790d202c2685f Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Thu, 28 Oct 2021 21:04:06 -0400 Subject: [PATCH] Remove harness file, arrayContains.js The `arrayContains` function has a number of deficiencies which make it inappropriate for Test262: - It apparently isn't very useful: despite being available for over 7 years, fewer than ten tests use it - It's misleading: its documentation reads, "Verify that a subArray is contained within an array." In reality, it only verifies that all the elements of one array are present in another--order does not matter. - It's not ergonomic for test authors: it has been misused to create tests that were prone to false positives [1] - It's not ergonomic for implementers: ostensibly designed for use with `assert`, the failure messages produced by tests that use it do not necessarily have very much context All code in the "harness" directory adds to the total amount of project-specific information which contributors are expected to to learn. In light of the above deficiencies, the burden of this particular harness file is unjustified. Remove the harness file and its associated tests. Update the tests which depend on it to express their expectations using alternate methods, and strengthen the tests to assert element order wherever appropriate. [1] https://github.com/tc39/test262/pull/3289 --- harness/arrayContains.js | 29 --------------- .../getOwnPropertyNames/15.2.3.4-4-2.js | 18 ++++++++-- .../built-ins/Temporal/getOwnPropertyNames.js | 28 +++++++-------- test/harness/arrayContains.js | 24 ------------- .../for-in/acquire-properties-from-array.js | 35 ++++++++++++++----- .../for-in/acquire-properties-from-object.js | 35 ++++++++++++++----- 6 files changed, 81 insertions(+), 88 deletions(-) delete mode 100644 harness/arrayContains.js delete mode 100644 test/harness/arrayContains.js diff --git a/harness/arrayContains.js b/harness/arrayContains.js deleted file mode 100644 index 5807320f1d..0000000000 --- a/harness/arrayContains.js +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (C) 2017 Ecma International. All rights reserved. -// This code is governed by the BSD license found in the LICENSE file. -/*--- -description: | - Verify that a subArray is contained within an array. -defines: [arrayContains] ----*/ - -/** - * @param {Array} array - * @param {Array} subArray - */ - -function arrayContains(array, subArray) { - var found; - for (var i = 0; i < subArray.length; i++) { - found = false; - for (var j = 0; j < array.length; j++) { - if (subArray[i] === array[j]) { - found = true; - break; - } - } - if (!found) { - return false; - } - } - return true; -} diff --git a/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-2.js b/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-2.js index 45cdcecdec..b94434ec89 100644 --- a/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-2.js +++ b/test/built-ins/Object/getOwnPropertyNames/15.2.3.4-4-2.js @@ -4,10 +4,22 @@ /*--- es5id: 15.2.3.4-4-2 description: Object.getOwnPropertyNames returns array of property names (Object) -includes: [arrayContains.js] ---*/ var result = Object.getOwnPropertyNames(Object); -var expResult = ["getPrototypeOf", "getOwnPropertyDescriptor", "getOwnPropertyNames", "create", "defineProperty", "defineProperties", "seal", "freeze", "preventExtensions", "isSealed", "isFrozen", "isExtensible", "keys", "prototype", "length"]; -assert(arrayContains(result, expResult), 'arrayContains(result, expResult) !== true'); +assert(result.indexOf("getPrototypeOf") > -1, "getPrototypeOf"); +assert(result.indexOf("getOwnPropertyDescriptor") > -1, "getOwnPropertyDescriptor"); +assert(result.indexOf("getOwnPropertyNames") > -1, "getOwnPropertyNames"); +assert(result.indexOf("create") > -1, "create"); +assert(result.indexOf("defineProperty") > -1, "defineProperty"); +assert(result.indexOf("defineProperties") > -1, "defineProperties"); +assert(result.indexOf("seal") > -1, "seal"); +assert(result.indexOf("freeze") > -1, "freeze"); +assert(result.indexOf("preventExtensions") > -1, "preventExtensions"); +assert(result.indexOf("isSealed") > -1, "isSealed"); +assert(result.indexOf("isFrozen") > -1, "isFrozen"); +assert(result.indexOf("isExtensible") > -1, "isExtensible"); +assert(result.indexOf("keys") > -1, "keys"); +assert(result.indexOf("prototype") > -1, "prototype"); +assert(result.indexOf("length") > -1, "length"); diff --git a/test/built-ins/Temporal/getOwnPropertyNames.js b/test/built-ins/Temporal/getOwnPropertyNames.js index 02de8fa0e9..783d933765 100644 --- a/test/built-ins/Temporal/getOwnPropertyNames.js +++ b/test/built-ins/Temporal/getOwnPropertyNames.js @@ -4,23 +4,19 @@ /*--- esid: sec-temporal-objects description: Temporal has own property names -includes: [arrayContains.js] features: [Temporal] ---*/ -const expected = [ - "Instant", - "TimeZone", - "PlainDate", - "PlainTime", - "PlainDateTime", - "ZonedDateTime", - "PlainYearMonth", - "PlainMonthDay", - "Duration", - "Calendar", - "Now", -]; const keys = Object.getOwnPropertyNames(Temporal); -assert.sameValue(keys.length, expected.length, "length"); -assert(arrayContains(keys, expected), "contents"); + +assert(keys.indexOf("Instant") > -1, "Instant"); +assert(keys.indexOf("TimeZone") > -1, "TimeZone"); +assert(keys.indexOf("PlainDate") > -1, "PlainDate"); +assert(keys.indexOf("PlainTime") > -1, "PlainTime"); +assert(keys.indexOf("PlainDateTime") > -1, "PlainDateTime"); +assert(keys.indexOf("ZonedDateTime") > -1, "ZonedDateTime"); +assert(keys.indexOf("PlainYearMonth") > -1, "PlainYearMonth"); +assert(keys.indexOf("PlainMonthDay") > -1, "PlainMonthDay"); +assert(keys.indexOf("Duration") > -1, "Duration"); +assert(keys.indexOf("Calendar") > -1, "Calendar"); +assert(keys.indexOf("Now") > -1, "Now"); diff --git a/test/harness/arrayContains.js b/test/harness/arrayContains.js deleted file mode 100644 index bc51ec0f0b..0000000000 --- a/test/harness/arrayContains.js +++ /dev/null @@ -1,24 +0,0 @@ -// Copyright (C) 2017 Rick Waldron. All rights reserved. -// This code is governed by the BSD license found in the LICENSE file. - -/*--- -description: > - Assert that the contents of an array contains another array as a slice or sub-array -includes: [arrayContains.js] ----*/ - -const willMatch = [0, 1, 2]; - -assert(arrayContains([0, 1, 2, 3], willMatch)); -assert(arrayContains([null, 0, 1, 2, 3], willMatch)); -assert(arrayContains([undefined, 0, 1, 2, 3], willMatch)); -assert(arrayContains([false, 0, 1, 2, 3], willMatch)); -assert(arrayContains([NaN, 0, 1, 2, 3], willMatch)); - -const willNotMatch = [1, 0, 2]; - -assert(!arrayContains([1, /* intentional hole */, 2], willNotMatch), '[1, /* intentional hole */, 2], willNotMatch)'); -assert(!arrayContains([1, null, 2], willNotMatch), '[1, null, 2], willNotMatch)'); -assert(!arrayContains([1, undefined, 2], willNotMatch), '[1, undefined, 2], willNotMatch)'); -assert(!arrayContains([1, false, 2], willNotMatch), '[1, false, 2], willNotMatch)'); -assert(!arrayContains([1, NaN, 2], willNotMatch), '[1, NaN, 2], willNotMatch)'); diff --git a/test/language/block-scope/syntax/for-in/acquire-properties-from-array.js b/test/language/block-scope/syntax/for-in/acquire-properties-from-array.js index 4fdb621e78..cee43a381d 100644 --- a/test/language/block-scope/syntax/for-in/acquire-properties-from-array.js +++ b/test/language/block-scope/syntax/for-in/acquire-properties-from-array.js @@ -4,19 +4,38 @@ es6id: 13.1 description: > for-in to acquire properties from array -includes: [arrayContains.js] ---*/ function props(x) { var array = []; for (let p in x) array.push(p); return array; } +var subject; -assert.sameValue(props([]).length, 0); -assert.sameValue(props([1]).length, 1); -assert.sameValue(props([1,2]).length, 2); -assert.sameValue(props([1,2,3]).length, 3); +subject = props([]); +assert.sameValue(subject.length, 0, "[]: length"); +assert.sameValue(subject[0], undefined, "[]: first property"); +assert.sameValue(subject[1], undefined, "[]: second property"); +assert.sameValue(subject[2], undefined, "[]: third property"); +assert.sameValue(subject[3], undefined, "[]: fourth property"); -assert(arrayContains(props([1]), ["0"])); -assert(arrayContains(props([1,2]), ["0", "1"])); -assert(arrayContains(props([1,2,3]), ["0", "1", "2"])); +subject = props([1]); +assert.sameValue(subject.length, 1, "[1]: length"); +assert.sameValue(subject[0], "0", "[1]: first property"); +assert.sameValue(subject[1], undefined, "[1]: second property"); +assert.sameValue(subject[2], undefined, "[1]: third property"); +assert.sameValue(subject[3], undefined, "[1]: fourth property"); + +subject = props([1, 2]); +assert.sameValue(subject.length, 2, "[1, 2]: length"); +assert.sameValue(subject[0], "0", "[1, 2]: first property"); +assert.sameValue(subject[1], "1", "[1, 2]: second property"); +assert.sameValue(subject[2], undefined, "[1, 2]: third property"); +assert.sameValue(subject[3], undefined, "[1, 2]: fourth property"); + +subject = props([1, 2, 3]); +assert.sameValue(subject.length, 3, "[1, 2, 3]: length"); +assert.sameValue(subject[0], "0", "[1, 2, 3]: first property"); +assert.sameValue(subject[1], "1", "[1, 2, 3]: second property"); +assert.sameValue(subject[2], "2", "[1, 2, 3]: third property"); +assert.sameValue(subject[3], undefined, "[1, 2, 3]: fourth property"); diff --git a/test/language/block-scope/syntax/for-in/acquire-properties-from-object.js b/test/language/block-scope/syntax/for-in/acquire-properties-from-object.js index ccd68eb6a7..bdf58ecfeb 100644 --- a/test/language/block-scope/syntax/for-in/acquire-properties-from-object.js +++ b/test/language/block-scope/syntax/for-in/acquire-properties-from-object.js @@ -4,19 +4,38 @@ es6id: 13.1 description: > for-in to acquire properties from object -includes: [arrayContains.js] ---*/ function props(x) { var array = []; for (let p in x) array.push(p); return array; } +var subject; -assert.sameValue(props({}).length, 0); -assert.sameValue(props({x:1}).length, 1); -assert.sameValue(props({x:1, y:2}).length, 2); -assert.sameValue(props({x:1, y:2, zoom:3}).length, 3); +subject = props({}); +assert.sameValue(subject.length, 0, "{}: length"); +assert.sameValue(subject[0], undefined, "{}: first property"); +assert.sameValue(subject[1], undefined, "{}: second property"); +assert.sameValue(subject[2], undefined, "{}: third property"); +assert.sameValue(subject[3], undefined, "{}: fourth property"); -assert(arrayContains(props({x:1}), ["x"])); -assert(arrayContains(props({x:1, y:2}), ["x", "y"])); -assert(arrayContains(props({x:1, y:2, zoom:3}), ["x", "y", "zoom"])); +subject = props({x:1}); +assert.sameValue(subject.length, 1, "{x:1}: length"); +assert.sameValue(subject[0], "x", "{x:1}: first property"); +assert.sameValue(subject[1], undefined, "{x:1}: second property"); +assert.sameValue(subject[2], undefined, "{x:1}: third property"); +assert.sameValue(subject[3], undefined, "{x:1}: fourth property"); + +subject = props({x:1, y:2}); +assert.sameValue(subject.length, 2, "{x:1, y:2}: length"); +assert.sameValue(subject[0], "x", "{x:1, y:2}: first property"); +assert.sameValue(subject[1], "y", "{x:1, y:2}: second property"); +assert.sameValue(subject[2], undefined, "{x:1, y:2}: third property"); +assert.sameValue(subject[3], undefined, "{x:1, y:2}: fourth property"); + +subject = props({x:1, y:2, zoom:3}); +assert.sameValue(subject.length, 3, "{x:1, y:2, zoom:3}: length"); +assert.sameValue(subject[0], "x", "{x:1, y:2, zoom:3}: first property"); +assert.sameValue(subject[1], "y", "{x:1, y:2, zoom:3}: second property"); +assert.sameValue(subject[2], "zoom", "{x:1, y:2, zoom:3}: third property"); +assert.sameValue(subject[3], undefined, "{x:1, y:2, zoom:3}: fourth property");