Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

RaisinTen
Copy link
Contributor

@RaisinTen RaisinTen added the blocked PRs that are blocked by other issues or PRs. label May 13, 2021
@github-actions 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
Copy link
Contributor

any reason why this PR still blocked?

@RaisinTen
Copy link
Contributor 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 RaisinTen marked this pull request as ready for review August 7, 2021 13:43
@RaisinTen RaisinTen force-pushed the test/replace-raw-errno-with-error-code branch from eb85c63 to 046c8ab Compare August 7, 2021 13:43
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@RaisinTen
Copy link
Contributor 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)

@nodejs-github-bot

This comment has been minimized.

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

cc @nodejs/platform-ibmi some help plz ^

@richardlau
Copy link
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
Copy link
Contributor Author

RaisinTen commented Aug 15, 2021 •
edited
Loading

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

@RaisinTen RaisinTen added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Aug 15, 2021
@RaisinTen 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
Copy link
Collaborator

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

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

@jasnell
Copy link
Member

jasnell commented Aug 26, 2021

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

@RaisinTen
Copy link
Contributor Author

RaisinTen commented Aug 27, 2021 •
edited
Loading

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
Copy link
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
Copy link
Member

jasnell commented Aug 28, 2021

Let's go ahead and land this.

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

PR-URL: #38675
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@jasnell
Copy link
Member

jasnell commented Aug 28, 2021

Landed in 21cf618

@jasnell jasnell closed this Aug 28, 2021
@RaisinTen 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
@RaisinTen @targos
test: use error code mapping in place of raw errno
bd80966 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
targos pushed a commit that referenced this pull request Sep 6, 2021
@RaisinTen @targos
test: use error code mapping in place of raw errno
007e285 
Refs: #38159 (comment)

PR-URL: #38675
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trivikram Kamat <[email protected]>
@targos targos mentioned this pull request Sep 6, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jatin1990gupta jatin1990gupta jatin1990gupta approved these changes

@trivikr trivikr trivikr approved these changes

@richardlau richardlau richardlau approved these changes

@cjihrig cjihrig cjihrig approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, 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
@RaisinTen @waelsy123 @nodejs-github-bot @richardlau @jasnell @addaleax @cjihrig @trivikr @jatin1990gupta