FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

test: use error code mapping in place of raw errno by RaisinTen · Pull Request #38675 · nodejs/node · GitHub

/ node Public

test: use error code mapping in place of raw errno - #38675

Closed
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:test/replace-raw-errno-with-error-code
ConversationCommits (1)ChecksFiles changed
Closed

test: use error code mapping in place of raw errno#38675
RaisinTen wants to merge 1 commit into
nodejs:masterfrom
RaisinTen:test/replace-raw-errno-with-error-code

Conversation

RaisinTen commented May 13, 2021

Copy link
Copy Markdown
Member

RaisinTen added the blocked PRs that are blocked by other issues or PRs. label May 13, 2021
github-actions Bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels May 13, 2021

waelsy123 commented May 22, 2021

Copy link
Copy Markdown
Contributor

any reason why this PR still blocked?

RaisinTen commented May 23, 2021

Copy link
Copy Markdown
Member Author

@waelsy123 This is still blocked because the linked PR hasn't landed in Node.js yet. We can unblock this after the next Libuv update PR lands here.

RaisinTen marked this pull request as ready for review August 7, 2021 13:43
RaisinTen force-pushed the test/replace-raw-errno-with-error-code branch from eb85c63 to 046c8ab Compare August 7, 2021 13:43

This comment has been minimized.

Sign in to view

RaisinTen commented Aug 7, 2021

Copy link
Copy Markdown
Member Author

How do I run an IBMi CI on this PR just like the one linked in this comment?

CI: https://ci.nodejs.org/job/node-test-commit-ibmi/329/
Test fs_read_type passed - https://ci.nodejs.org/job/node-test-commit-ibmi/329/nodes=ibmi72-ppc64/testReport/(root)/test/parallel_test_fs_read_type/

Originally posted by @dmabupt in #38159 (comment)

This comment has been minimized.

Sign in to view
RaisinTen removed the blocked PRs that are blocked by other issues or PRs. label Aug 7, 2021

RaisinTen commented Aug 14, 2021

Copy link
Copy Markdown
Member Author

cc @nodejs/platform-ibmi some help plz ^

richardlau commented Aug 14, 2021

Copy link
Copy Markdown
Member

How do I run an IBMi CI on this PR just like the one linked in this comment?

CI: https://ci.nodejs.org/job/node-test-commit-ibmi/329/

https://ci.nodejs.org/job/node-test-commit-ibmi/ is like any other node-test-commit-* job (except for the moment it is not run on every PR because it takes much longer to run on our current machines) -- follow the guide in https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#starting-a-jenkins-ci-job

RaisinTen commented Aug 15, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

IBM i CI: https://ci.nodejs.org/job/node-test-commit-ibmi/493/

RaisinTen added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Aug 15, 2021
RaisinTen force-pushed the test/replace-raw-errno-with-error-code branch from 046c8ab to 7c8156d Compare August 22, 2021 12:58

nodejs-github-bot commented Aug 22, 2021 •
edited by jasnell
Loading

Copy link
Copy Markdown
Collaborator

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

jasnell commented Aug 26, 2021

Copy link
Copy Markdown
Member

@RaisinTen ... I notice your comment about this being blocked before. Is it still blocked at all?

RaisinTen commented Aug 27, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Hey @jasnell, this is not blocked anymore. I was just trying to get a green IBM i CI on this PR as the changed code path can only be hit on an IBM i. I've been updating #38675 (comment) every time I started a new rebuild but unfortunately, I'm consistently getting a number of ECONNRESETs. Is it worth it to wait for a green CI or should we just land this?

richardlau commented Aug 27, 2021

Copy link
Copy Markdown
Member

I'm consistently getting a number of ECONNRESETs.

Those started appearing after the recent libuv update and are being tracked in #39683. I did make @nodejs/platform-ibmi aware of them.

jasnell commented Aug 28, 2021

Copy link
Copy Markdown
Member

Let's go ahead and land this.

jasnell pushed a commit that referenced this pull request Aug 28, 2021
test: use error code mapping in place of raw errno
21cf618 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>

jasnell commented Aug 28, 2021

Copy link
Copy Markdown
Member

Landed in 21cf618

jasnell closed this Aug 28, 2021
RaisinTen deleted the test/replace-raw-errno-with-error-code branch August 29, 2021 01:03
targos pushed a commit that referenced this pull request Sep 6, 2021
test: use error code mapping in place of raw errno
007e285 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
targos mentioned this pull request Sep 6, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
cjihrig cjihrig approved these changes
richardlau richardlau approved these changes
trivikr trivikr approved these changes
+1 more reviewer
jatin1990gupta jatin1990gupta approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, and a CI started. needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL