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: skip test-net-connect-reset-until-connected on SmartOS #43449

Merged
merged 1 commit into from
Jun 19, 2022
Merged

test: skip test-net-connect-reset-until-connected on SmartOS #43449

merged 1 commit into from
Jun 19, 2022

Conversation

panva
Copy link
Member

@panva panva commented Jun 16, 2022

Refs: #43446

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jun 16, 2022
@panva panva added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jun 16, 2022
@panva panva requested a review from lpinca June 16, 2022 09:23
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
lpinca
lpinca approved these changes Jun 16, 2022
View reviewed changes
Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

I'm fine with this but it would be nice the find the cause.

@panva panva added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 16, 2022
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@richardlau
Copy link
Member

Nit: marking a test flaky doesn't skip it -- it changes how we treat failures for it.

@panva
Copy link
Member Author

panva commented Jun 16, 2022

Nit: marking a test flaky doesn't skip it -- it changes how we treat failures for it.

I guess the nit is for the commit message then? Shall I update it and re-run CI?

@nodejs-github-bot
Copy link
Collaborator

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

@panva panva added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 16, 2022
@lpinca
Copy link
Member

lpinca commented Jun 16, 2022

As per #43446 (comment) and #43446 (comment) other tests are also timing out on "smartos20-64". I wonder if it makes sense to restart the machine.

@panva panva removed the fast-track PRs that do not need to wait for 48 hours to land. label Jun 16, 2022
@nodejs nodejs deleted a comment from github-actions bot Jun 16, 2022
@github-actions github-actions bot mentioned this pull request Jun 17, 2022
Open
31 tasks
@bnoordhuis
Copy link
Member

FYI, I'm proposing in #43457 to simply drop smartos support.

@panva
Copy link
Member Author

panva commented Jun 17, 2022 •
edited
Loading

@bnoordhuis That's fine, but until then can we unjam CI/PRs by marking this test as flaky?

@panva panva added the fast-track PRs that do not need to wait for 48 hours to land. label Jun 17, 2022
@github-actions
Copy link
Contributor

Fast-track has been requested by @panva. Please 👍 to approve.

This was referenced Jun 18, 2022
Open
Open
@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 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 Jun 19, 2022
@nodejs-github-bot
Copy link
Collaborator

Commit Queue failed
- Loading data for nodejs/node/pull/43449
✔  Done loading data for nodejs/node/pull/43449
----------------------------------- PR info ------------------------------------
Title      test: skip test-net-connect-reset-until-connected on SmartOS (#43449)
Author     Filip Skokan  (@panva)
Branch     panva:flaky-test-net-connect-reset-until-connected -> nodejs:main
Labels     test, flaky-test, fast-track, author ready, needs-ci
Commits    1
 - test: skip test-net-connect-reset-until-connected on SmartOS
Committers 1
 - Filip Skokan 
PR-URL: https://github.com/nodejs/node/pull/43449
Refs: https://github.com/nodejs/node/issues/43446
Reviewed-By: Luigi Pinca 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/43449
Refs: https://github.com/nodejs/node/issues/43446
Reviewed-By: Luigi Pinca 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 16 Jun 2022 09:22:01 GMT
   ✔  Approvals: 1
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/43449#pullrequestreview-1008839474
   ℹ  This PR is being fast-tracked
   ✖  The fast-track request requires at least two collaborators' approvals (👍).
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2022-06-16T12:03:05Z: https://ci.nodejs.org/job/node-test-pull-request/44616/
- Querying data for job/node-test-pull-request/44616/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/2523244812

@aduh95 aduh95 removed the fast-track PRs that do not need to wait for 48 hours to land. label Jun 19, 2022
@aduh95 aduh95 removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Jun 19, 2022
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2022
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 19, 2022
@nodejs-github-bot nodejs-github-bot merged commit 2609dc6 into nodejs:main Jun 19, 2022
@nodejs-github-bot
Copy link
Collaborator

Landed in 2609dc6

targos pushed a commit that referenced this pull request Jul 12, 2022
@panva @targos
test: skip test-net-connect-reset-until-connected on SmartOS
b1a7798 
Refs: #43446

PR-URL: #43449
Refs: #43446
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@targos targos mentioned this pull request Jul 12, 2022
Merged
targos pushed a commit that referenced this pull request Jul 31, 2022
@panva @targos
test: skip test-net-connect-reset-until-connected on SmartOS
7cb71c0 
Refs: #43446

PR-URL: #43449
Refs: #43446
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@targos targos mentioned this pull request Aug 3, 2022
Merged
guangwong pushed a commit to noslate-project/node that referenced this pull request Oct 10, 2022
@panva @guangwong
test: skip test-net-connect-reset-until-connected on SmartOS
b067b9a 
Refs: #43446

PR-URL: nodejs/node#43449
Refs: nodejs/node#43446
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@panva panva deleted the flaky-test-net-connect-reset-until-connected branch October 13, 2022 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 approved these changes

@lpinca lpinca lpinca 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. flaky-test Issues and PRs related to the tests with unstable failures on the CI. 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.
6 participants
@panva @nodejs-github-bot @richardlau @lpinca @bnoordhuis @aduh95