Skip to content

Conversation

@benjamingr
Copy link
Member

Add some test262 tests for every, add some length checks.

Turns out properties need to be writable after all according to the
test262 tests.

cc @nodejs/streams @aduh95

@benjamingr benjamingr added the stream Issues and PRs related to the stream subsystem. label Feb 14, 2022
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Feb 14, 2022
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022
@nodejs-github-bot
Copy link
Collaborator

}

async function some(fn, options) {
async function some(fn, options = {}) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to create a throw away object.

Suggested change
async function some(fn, options = {}) {
async function some(fn, options = undefined) {

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@benjamingr benjamingr force-pushed the add-more-test262-tests branch from 9644df7 to ebc476f Compare February 23, 2022 10:36
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2022
@nodejs-github-bot
Copy link
Collaborator

Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.
@benjamingr benjamingr force-pushed the add-more-test262-tests branch from ebc476f to c8bf95a Compare February 23, 2022 11:37
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2022
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 23, 2022
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@benjamingr benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2022
@nodejs-github-bot nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Feb 23, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/41974
✔  Done loading data for nodejs/node/pull/41974
----------------------------------- PR info ------------------------------------
Title      stream: port more test262 tests (#41974)
Author     Benjamin Gruenbaum  (@benjamingr)
Branch     benjamingr:add-more-test262-tests -> nodejs:master
Labels     stream, needs-ci
Commits    1
 - stream: port more test262 tests
Committers 1
 - Benjamin Gruenbaum 
PR-URL: https://github.com/nodejs/node/pull/41974
Reviewed-By: James M Snell 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41974
Reviewed-By: James M Snell 
Reviewed-By: Robert Nagy 
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - stream: port more test262 tests
   ℹ  This PR was created on Mon, 14 Feb 2022 20:47:25 GMT
   ✔  Approvals: 3
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41974#pullrequestreview-887813994
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/41974#pullrequestreview-887815061
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41974#pullrequestreview-887906150
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-23T18:10:07Z: https://ci.nodejs.org/job/node-test-pull-request/42747/
- Querying data for job/node-test-pull-request/42747/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/1889467631

@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Feb 23, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 23, 2022
@nodejs-github-bot nodejs-github-bot merged commit fd3997b into nodejs:master Feb 23, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in fd3997b

sxa pushed a commit to sxa/node that referenced this pull request Mar 7, 2022
Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.

PR-URL: nodejs#41974
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@sxa sxa mentioned this pull request Mar 8, 2022
xtx1130 pushed a commit to xtx1130/node that referenced this pull request Apr 25, 2022
Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.

PR-URL: nodejs#41974
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@juanarbol
Copy link
Member

This PR can not be landed in V16.x due to a dependency with #41775

targos pushed a commit that referenced this pull request Jul 12, 2022
Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.

PR-URL: #41974
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this pull request Jul 31, 2022
Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.

PR-URL: #41974
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
Add some test262 tests for `every`, add some `length` checks.

Turns out properties need to be writable after all according to the
test262 tests.

PR-URL: nodejs/node#41974
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants