stream: add more filter tests by benjamingr · Pull Request #41936 · nodejs/node

benjamingr · 2022-02-11T17:55:59Z

Add more tests to Readable.prototype.filter

cc @nodejs/streams @aduh95

nodejs-github-bot · 2022-02-12T17:28:10Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42515/

aduh95 · 2022-02-12T17:50:25Z

test/parallel/test-stream-filter.js

+  });
+  assert.rejects(
+    stream.filter(() => true).toArray(),
+    /boom/,


I think we would want to check the error by reference here, to match the spec proposal text.

I recall this discussion before but I don't recall the conclusion, do you happen to have a reference?

#41642 (comment)

mcollina

lgtm

nodejs-github-bot · 2022-02-13T16:59:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42520/

nodejs-github-bot · 2022-02-13T19:18:19Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42525/

nodejs-github-bot · 2022-02-13T21:31:50Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42529/

nodejs-github-bot · 2022-02-13T22:18:01Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42533/

aduh95 · 2022-02-13T23:25:01Z

There's a CI failure related to the changes in this PR:

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

4 !== 2

    at Immediate._onImmediate (/home/iojs/build/workspace/node-test-binary-arm/test/parallel/test-stream-filter.js:136:12)
    at processImmediate (node:internal/timers:466:21) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 4,
  expected: 2,
  operator: 'strictEqual'
}

benjamingr · 2022-02-14T07:33:36Z

@aduh95 that's actually not related to this PR - but I can see why that'd happen I'll push a fix (which again is unrelated to this PR).

nodejs-github-bot · 2022-02-14T07:44:18Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42544/

nodejs-github-bot · 2022-02-14T11:58:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42548/

nodejs-github-bot · 2022-02-14T14:26:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/42554/

nodejs-github-bot · 2022-02-14T16:15:07Z

Commit Queue failed

- Loading data for nodejs/node/pull/41936
✔  Done loading data for nodejs/node/pull/41936
----------------------------------- PR info ------------------------------------
Title      stream: add more filter tests (#41936)
Author     Benjamin Gruenbaum  (@benjamingr)
Branch     benjamingr:add-filter-tests -> nodejs:master
Labels     test, needs-ci
Commits    3
 - stream: add more filter tests
 - fixup!
 - fixup! remove timeout from test
Committers 1
 - Benjamin Gruenbaum 
PR-URL: https://github.com/nodejs/node/pull/41936
Reviewed-By: Robert Nagy 
Reviewed-By: James M Snell 
Reviewed-By: Matteo Collina 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/41936
Reviewed-By: Robert Nagy 
Reviewed-By: James M Snell 
Reviewed-By: Matteo Collina 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - fixup! remove timeout from test
   ℹ  This PR was created on Fri, 11 Feb 2022 17:55:59 GMT
   ✔  Approvals: 3
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/41936#pullrequestreview-880871183
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/41936#pullrequestreview-880872056
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/41936#pullrequestreview-880887559
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-02-14T14:26:06Z: https://ci.nodejs.org/job/node-test-pull-request/42554/
- Querying data for job/node-test-pull-request/42554/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1842083455

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

benjamingr · 2022-02-14T16:25:21Z

Forgot to autosquash, meh, landed manually :)

Landed in da11381 🎉

aduh95 · 2022-02-14T17:04:07Z

Forgot to autosquash, meh

FYI you can use commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. to ask the CQ to squash all the commits when landing a PR.

benjamingr · 2022-02-14T17:18:39Z

Thanks, yeah, I forgot to use that and already had a terminal open with node so I just landed with ncu :)

PR-URL: nodejs#41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

danielleadams · 2022-04-19T16:06:39Z

@benjamingr this breaks when landing on v16.x-staging. Can you open a backport PR to land on v16.x? Thank you

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

PR-URL: nodejs/node#41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

stream: add more filter tests

b3ccbce

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Feb 11, 2022

fixup!

6d10c20

benjamingr mentioned this pull request Feb 12, 2022

stream: add more forEach tests #41937

Closed

ronag approved these changes Feb 12, 2022

View reviewed changes

jasnell approved these changes Feb 12, 2022

View reviewed changes

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 12, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 12, 2022

aduh95 reviewed Feb 12, 2022

View reviewed changes

mcollina approved these changes Feb 12, 2022

View reviewed changes

github-actions bot mentioned this pull request Feb 13, 2022

CI Reliability 2022-02-13 nodejs/reliability#201

Open

19 tasks

github-actions bot mentioned this pull request Feb 14, 2022

CI Reliability 2022-02-14 nodejs/reliability#202

Open

18 tasks

fixup! remove timeout from test

3e39e50

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 14, 2022

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 14, 2022

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 14, 2022

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Feb 14, 2022

benjamingr added a commit that referenced this pull request Feb 14, 2022

stream: add more filter tests

da11381

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

benjamingr closed this Feb 14, 2022

This was referenced Feb 15, 2022

CI Reliability 2022-02-15 nodejs/reliability#203

Open

CI Reliability 2022-02-16 nodejs/reliability#204

Open

CI Reliability 2022-02-17 nodejs/reliability#205

Open

bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022

stream: add more filter tests

8949daa

PR-URL: nodejs#41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

bengl pushed a commit to bengl/node that referenced this pull request Feb 21, 2022

stream: add more filter tests

db2dc99

PR-URL: nodejs#41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

bengl mentioned this pull request Feb 21, 2022

v17.6.0 release proposal #42072

Merged

bengl pushed a commit that referenced this pull request Feb 21, 2022

stream: add more filter tests

6c9a36c

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

bengl pushed a commit that referenced this pull request Feb 22, 2022

stream: add more filter tests

f5d7339

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

danielleadams added the backport-requested-v16.x label Apr 19, 2022

targos removed the backport-requested-v16.x label Jul 31, 2022

targos pushed a commit that referenced this pull request Jul 31, 2022

stream: add more filter tests

c4dc8b6

PR-URL: #41936 Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Uh oh!

Conversation

benjamingr commented Feb 11, 2022

Uh oh!

nodejs-github-bot commented Feb 12, 2022

Uh oh!

aduh95 Feb 12, 2022

Choose a reason for hiding this comment

Uh oh!

benjamingr Feb 12, 2022

Choose a reason for hiding this comment

Uh oh!

aduh95 Feb 12, 2022

Choose a reason for hiding this comment

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Feb 13, 2022

Uh oh!

nodejs-github-bot commented Feb 13, 2022

Uh oh!

nodejs-github-bot commented Feb 13, 2022

Uh oh!

nodejs-github-bot commented Feb 13, 2022

Uh oh!

aduh95 commented Feb 13, 2022

Uh oh!

benjamingr commented Feb 14, 2022

Uh oh!

nodejs-github-bot commented Feb 14, 2022

Uh oh!

nodejs-github-bot commented Feb 14, 2022

Uh oh!

nodejs-github-bot commented Feb 14, 2022

Uh oh!

nodejs-github-bot commented Feb 14, 2022

Uh oh!

benjamingr commented Feb 14, 2022

Uh oh!

aduh95 commented Feb 14, 2022

Uh oh!

benjamingr commented Feb 14, 2022

Uh oh!

danielleadams commented Apr 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants