stream: add more filter tests #41936

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

PR-URL: #41936 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

PR-URL: nodejs/node#41936 Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Matteo Collina <[email protected]>

targos mentioned this pull request Aug 3, 2022

v16.17.0 release proposal #44098

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

stream: add more filter tests #41936

stream: add more filter tests #41936

benjamingr commented Feb 11, 2022

nodejs-github-bot commented Feb 12, 2022

aduh95 Feb 12, 2022

benjamingr Feb 12, 2022

aduh95 Feb 12, 2022

mcollina left a comment

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

aduh95 commented Feb 13, 2022

benjamingr commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

benjamingr commented Feb 14, 2022

aduh95 commented Feb 14, 2022

benjamingr commented Feb 14, 2022

danielleadams commented Apr 19, 2022

stream: add more filter tests #41936

stream: add more filter tests #41936

Conversation

benjamingr commented Feb 11, 2022

nodejs-github-bot commented Feb 12, 2022

aduh95 Feb 12, 2022

Choose a reason for hiding this comment

benjamingr Feb 12, 2022

Choose a reason for hiding this comment

aduh95 Feb 12, 2022

Choose a reason for hiding this comment

mcollina left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

nodejs-github-bot commented Feb 13, 2022

aduh95 commented Feb 13, 2022

benjamingr commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

nodejs-github-bot commented Feb 14, 2022

benjamingr commented Feb 14, 2022

aduh95 commented Feb 14, 2022

benjamingr commented Feb 14, 2022

danielleadams commented Apr 19, 2022